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

setup_pygrb_minifollowups cleaner #4872

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

pannarale
Copy link
Contributor

@pannarale pannarale commented Sep 11, 2024

Small cleanup of setup_pygrb_minifollowups

Standard information about the request

This change affects: PyGRB

This change will: remove an unused variable (trig_time) and make the trigger file (trigger_file) a mandatory argument of setup_pygrb_minifollowups.

Motivation

This is the first PR to breakdown the workflow changes that produce a PyGRB results page with injections, exclusion distances, open box, followups of missed injections, offsource and onsource triggers, timeslides, etc.

Testing performed

See link above to the most recent results webpage.

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

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.

A minor request, happy for this to be merged once addressed.

@@ -710,14 +710,12 @@ def setup_pygrb_minifollowups(workflow, followups_file,
tags=tags)
node = exe.create_node()

node.add_input_opt('--trig-file', resolve_url_to_file(trigger_file))
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, the resolve_url_to_file call would not happen here, but instead be done as soon as the trigger_file is read in (presumably a few levels above this). The help states that trigger_file is a pycbc.workflow.File, which is what we would want, but not what seems to be happening if this is needed.

@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 83fd3a7 into gwastro:master Sep 19, 2024
30 checks passed
@pannarale pannarale deleted the cleaner_grb_minifollowups branch September 19, 2024 16:36
prayush pushed a commit to prayush/pycbc that referenced this pull request Nov 21, 2024
* setup_pygrb_minifollowups cleaner

* Removing resolve_url_to_file from setup_pygrb_minifollowups
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