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

Enable handling of all pycbc_pygrb_efficiency in workflow #4873

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

pannarale
Copy link
Contributor

@pannarale pannarale commented Sep 11, 2024

Standard information about the request

This is the second of the PRs promised in #4872.

It brings the PyGRB workflow utilities up to date with the output produced pycbc_pygrb_efficiency. Specifically, this script now generates a json file with exclusion distances, which the (post processing) workflow generator needs to catch and handle appropriately. The json file is intended to be used by the executable introduce in #4756 to display exclusion distances in a table.

This change affects: PyGRB

This change changes: presentation of scientific output

This change will come to full circle with another PR to invoke and display the generation of the exclusion distances table.

Links to any issues or associated PRs

This is the 2nd PR of a series opened by #4872 but it does not depend on it.

Testing performed

A full results webpage is available but it requires more changes than this one to produce.

  • The author of this pull request confirms they will adhere to the code of conduct

@pannarale pannarale added the PyGRB PyGRB development label Sep 11, 2024
@pannarale pannarale requested a review from spxiwh September 11, 2024 10:34
@pannarale pannarale self-assigned this Sep 11, 2024
Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

Minor comment here, but happy to merge once considered.

@@ -539,6 +543,8 @@ def make_pygrb_plot(workflow, exec_name, out_dir,
node.add_opt('--y-variable', tags[0])
# Quantity to be displayed on the x-axis of the plot
elif exec_name == 'pygrb_plot_stats_distribution':
seg_filelist = FileList([resolve_url_to_file(sf) for sf in seg_files])
Copy link
Contributor

Choose a reason for hiding this comment

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

As with the other PR, we should not be passing around URLs (or paths) to files, but passing around File objects wherever possible. If seg_files is read in the top-level workflow generator, it would be better to immediately convert to a File object.

@pannarale
Copy link
Contributor Author

@spxiwh, I followed up on your comments, thanks. I reran the post-processing portion of the run and successfully produced a new webpage which looks like the original one.

@pannarale pannarale merged commit 6d31478 into gwastro:master Sep 19, 2024
30 checks passed
@pannarale pannarale deleted the make_pygrb_plot_update branch September 19, 2024 16:37
prayush pushed a commit to prayush/pycbc that referenced this pull request Nov 21, 2024
* Enable handling json file output from pycbc_pygrb_efficiency in pygrb workflow

* Removing resolve_url_to_file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PyGRB PyGRB development
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants