-
Notifications
You must be signed in to change notification settings - Fork 364
Adding injections and bank corner plots to pyGRB workflows #5145
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
base: master
Are you sure you want to change the base?
Conversation
@@ -468,6 +481,25 @@ layout.two_column_layout(coinc_out_dir, coinc_files) | |||
# | |||
out_dir = rdir['injections'] | |||
_workflow.makedir(out_dir) | |||
## Create corner plot | |||
files = _workflow.FileList([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend a more specific name for this variable, as this variable name is already used above. I think inj_corner_plot_files
is good.
plot_node, bank_plot = _workflow.make_pygrb_plot( | ||
wflow, 'pycbc_plot_bank_corner', out_dir, bank_file=bank_file | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you also need to append plot_node
to plotting_nodes
, as done in the lines above for the sky grid plot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right; I wonder why this works, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess in your test, the job may have completed early enough to hide the missing dependency. But we might not always be so lucky [1], so it best to do this properly.
params = workflow.cp.get_opt_tags(exec_name, 'parameters', ['injs']) | ||
else: | ||
node.add_opt('--title', f'\"Template bank\"') | ||
params = workflow.cp.get_opt_tags(exec_name, 'parameters', ['bank']) | ||
node.add_opt('--parameters', params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think you need to handle the parameter list explicitly here. Any option given in the executable's section via the config file will be passed straight as a command line option by the workflow base code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is needed for the subsections (i.e. pycbc_plot_bank_corner_NSBH
). And we need to pass the parameters separately because injections and bank plotting have different parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I am aware, the functionality you describe comes "for free" already. In the config file you will have two sections
[plot_corner-bank]
parameters = mass1 mass2 spin1z spin2z
title = "Template bank"
no-suptitle =
[plot_corner-bnsinj]
parameters = mass1 mass2 distance inclination ra dec
title = "Injections"
no-suptitle =
When you instantiate PlotExecutable
and call create_node()
above, this should automatically pick up the right section and all the arguments therein. I do not think you need to explicitly call add_opt()
.
As a broader comment, I suggest not using |
Co-authored-by: Tito Dal Canton <[email protected]>
node.add_opt('--title', f'\"{tags[0]} injections\"') | ||
params = workflow.cp.get_opt_tags(exec_name, 'parameters', ['injs']) | ||
else: | ||
node.add_opt('--title', f'\"Template bank\"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node.add_opt('--title', f'\"Template bank\"') | |
node.add_opt('--title', '\"Template bank\"') |
This change affects: PyGRB
This change changes: result presentation / plotting
Summary:
Please see this for the current implementation.
This also addresses one of the tasks in #3660