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

Enforce pygrb_efficiency subsections #4909

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

pannarale
Copy link
Contributor

  • Require one pygrb_efficiency subsection in the config file for each injection set, i.e. [injections] subsection.
  • Insert a line that was accidentally removed
  • Lower the high end limit of the x axis in pygrb_efficiency plots.

Standard information about the request

This is a: bug fix. Prior to this fix, pygrb_efficiency was using a single value of lower/upper distance values, regardless of the injection set.

This change affects: PyGRB

This change changes: result presentation plotting and scientific output

This change was tested by rerunning the results generation of a GRB analysis here. Note that the previous version of plots in this section was "jagged".

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

…iguration file and insert back a line previously delete unintentionally
@pannarale pannarale added the PyGRB PyGRB development label Oct 12, 2024
@pannarale pannarale requested a review from spxiwh October 12, 2024 13:58
@pannarale pannarale self-assigned this Oct 12, 2024
err_msg += "corresponding [pygrb_efficiency] subsection. "
err_msg += "[injections] subsections found: %s. "
err_msg += "[pygrb_efficiency] subsections found: %s. "
logging.error(err_msg, inj_sets, eff_secs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, there might be use cases where these subsections wouldn't be needed (e.g. running only one injection set, or just running a NS injection set ... maybe some new theory proves next year that NSBH GRBs definitely don't exist), but if it's better to enforce this, I won't object

Enforcing the two lists be equal is possibly not what you want, there could be other subsections in efficiency that are not injection names. I think you really want to check that inj_sets is a subset of eff_secs.

if set(inj_sets).issubset(eff_secs):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I see your point. I may want to make more efficiency plot versions, so I need at least enough eff_secs to cover the inj_sets. I will make this change shortly.

@pannarale pannarale merged commit 2c6c60d into gwastro:master Oct 18, 2024
30 checks passed
@pannarale pannarale deleted the efficiency_subsections branch October 18, 2024 07:58
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