Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow option for workflow to end at inspiral jobs #4612

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

kkacanja
Copy link
Contributor

@kkacanja kkacanja commented Jan 27, 2024

Added a the option stop-after under [workflow] where you can supply the options ['inspiral', 'hdf_trigger_merge', 'statmap'] for the time being to only do the workflow up until the specified job.

This is useful for clusters such as OSG where inspiral jobs can be parallelized to one core jobs and be parallelized over many nodes. Single processing is not ideal on OSG so this option is useful for these types of clusters.

This does not take into account injection inspiral jobs.

Also, when using these options, make sure to change the file-retention level in [workflow] such that your files do not get deleted when the workflow finishes running.

@kkacanja kkacanja force-pushed the kkacanja-patch branch 4 times, most recently from aeba20e to b18a2c3 Compare January 31, 2024 22:52
@@ -187,8 +188,76 @@ insps = wf.merge_single_detector_hdf_files(workflow, hdfbank,
insps, output_dir,
tags=['full_data'])

# Check for the only-do section. If inspiral=true then it will only do the workflow up until the merged inspiral hdf files
if 'only-do' in workflow.cp.sections():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option may need a better name so it is clear in the config file. Maybe something more verbose?

else:
only_do_inspiral = False

if only_do_inspiral:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like we could combine with the code at the end to remove the redundance here. Maybe put into a function and then we can simply call from either location?

@ahnitz
Copy link
Member

ahnitz commented Feb 1, 2024

As further clarification, this is useful in the case where you will do all the post-processing at a different cluster. Once can finish the computationally expensive parts on the OSG and then transfer the data to a local cluster to do the later workflow stages.

@spxiwh
Copy link
Contributor

spxiwh commented Feb 2, 2024

Thanks @kkacanja . A couple of comments/requests from me on this:

  • The proposed [only-do] section of the config file breaks the configfile format (https://pycbc.org/pycbc/latest/html/workflow/initialization.html). This should be within the [workflow] section in some way. This isn't really only-do, but is stop-after and there's probably only 2 or 3 logical choices for that in this workflow, so just adding a stop-after-inspirals= into the [workflow] section is my suggestion. stop-after-statmap would also be another natural stopping point.
  • If I ask the workflow to do the inspiral jobs, it should do all the inspiral jobs, including injection runs. Currently this patch will not run injection inspiral jobs.
  • As Alex, said, we need to avoid code duplication with the "stop and write the workflow to file" block.

Maybe the easiest way to achieve all of this is to put large blocks of the code into a number of functions in the executable and don't call some of them (or return early from the function) if this option is given. The code is already split up into logical blocks: do main filtering, do plots, do injection filtering, do injection plots, etc. Each of these could be made a function.

@kkacanja
Copy link
Contributor Author

kkacanja commented Feb 2, 2024

@spxiwh @ahnitz Thanks for all your comments. I agree with all the changes that should be made and I'll work on them.

@@ -183,6 +257,17 @@ ind_insps = insps = wf.setup_matchedfltr_workflow(workflow, analyzable_segs,
datafind_files, splitbank_files_fd,
output_dir, tags=['full_data'])


if workflow.cp.has_option('workflow', 'stop-after-inspiral'):
stop_after_inspiral = workflow.cp.getboolean('workflow', 'stop-after-inspiral')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this

stop-after = inspiral (statmap, etc). 

Copy link
Member

@ahnitz ahnitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkacanja Change the one thing noted, and test locally. If that works, I think this can then be merged. Adding additional stop points as people need can be left to the future. I think adding one for inspiral and statmap are fine here.

layout.single_layout(rdir['workflow'], ([dashboard_file, gen_file_html, log_file_html]))
sys.exit(0)

def check_stop(job_name, container, workflow, finalize_workflow):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to add a short comment here to explain what the function does.

@ahnitz ahnitz merged commit a3f5e92 into gwastro:master Feb 27, 2024
31 of 33 checks passed
bhooshan-gadre pushed a commit to bhooshan-gadre/pycbc that referenced this pull request Mar 4, 2024
* Updates to stopping after data inspiral jobs and statmap

* Updates to stopping after data inspiral jobs and statmap

* Changed typing of stop-after

* Allow pycbc_make_offline_search_workflow to stop after inspiral jobs.

* Fixed comment
lpathak97 pushed a commit to lpathak97/pycbc that referenced this pull request Mar 13, 2024
* Updates to stopping after data inspiral jobs and statmap

* Updates to stopping after data inspiral jobs and statmap

* Changed typing of stop-after

* Allow pycbc_make_offline_search_workflow to stop after inspiral jobs.

* Fixed comment
acorreia61201 pushed a commit to acorreia61201/pycbc that referenced this pull request Apr 4, 2024
* Updates to stopping after data inspiral jobs and statmap

* Updates to stopping after data inspiral jobs and statmap

* Changed typing of stop-after

* Allow pycbc_make_offline_search_workflow to stop after inspiral jobs.

* Fixed comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants