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

Function to setup pygrb_grb_info_table jobs #4875

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

pannarale
Copy link
Contributor

@pannarale pannarale commented Sep 11, 2024

Standard information about the request

This PR expands the scope of make_info_table so it can add jobs to the workflow for pygrb_exclusion_dist_table as well as pygrb_grb_info_table.

This is a: a new feature for PyGRB workflow generation.

This change allows to expand presentation of scientific output in PyGRB webpages

Links to any issues or associated PRs

This change is the 3rd PR of a series started with #4872.

Testing performed

As for the previous two PRs an example of a complete webpage is here, but the full webpage will be reproducible only once all workflow changes are on master.

  • 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 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.

A couple of stylistic comments, but nothing that should hold things up.

@@ -560,30 +560,37 @@ def make_pygrb_plot(workflow, exec_name, out_dir,
return node, node.output_files


def make_info_table(workflow, out_dir, tags=None):
"""Setup a job to create an html snippet with the GRB trigger information.
def make_info_table(workflow, exec_name, out_dir, in_files=None, tags=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the function be a bit more specific make_grb_info_table at least?

workflow.cp.get('workflow', 'trigger-time'))
node.add_opt('--ra', workflow.cp.get('workflow', 'ra'))
node.add_opt('--dec', workflow.cp.get('workflow', 'dec'))
node.add_opt('--sky-error', workflow.cp.get('workflow', 'sky-error'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do these need to be read from cp.get? The all-sky workflow has numerous cases where options are shared between codes, and the config files have syntax to support this .... The idea being to avoid these setup codes being unecessarily complex and having the configs specifying, as much as possible, what arguments codes will run with.

@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 82d6263 into gwastro:master Sep 19, 2024
30 checks passed
@pannarale pannarale deleted the enable_pygrb_excl_dist_tables branch September 19, 2024 16:37
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