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

Use approximant parsing argument for make_bayestar #4712

Conversation

GarethCabournDavies
Copy link
Contributor

@GarethCabournDavies GarethCabournDavies commented Apr 23, 2024

When generating bayestar skymaps in the upload prep minifollowup, the approximant used was not the same as in the rest of the workflow, this caused issues.

Here we use the standard approximant interface used by other codes in order to work out the appropriate waveform.

Standard information about the request

This is a: bug fix
This change affects: the offline search
This change changes: result presentation / plotting, scientific output

This change: follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change will: require a new release on the v23 release branch so that it can be used for o4 LVK results

Motivation

The correct approximant was not being used for the bayestar subprocess, this means the skymaps would be wrong

Contents

I have used standard library code for obtaining the approximant from the command line interface given conditions, rather than only allowing one input.

Links to any issues or associated PRs

None

Testing performed

Previously failing event now runs, with the right approximant. Boundary also moved to ensure that the SPAtmplt --> TaylorF2 check was implemented

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

@GarethCabournDavies GarethCabournDavies added bug offline search v23_release_branch PRs applied to the v2.3.X release branch or to be cherry-picked if merging to master labels Apr 23, 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.

Looks good, assuming it works!

@GarethCabournDavies GarethCabournDavies merged commit 9b8cbc5 into gwastro:master Apr 29, 2024
33 checks passed
@GarethCabournDavies GarethCabournDavies deleted the make_bayestar_approximant branch April 29, 2024 08:14
ArthurTolley pushed a commit to ArthurTolley/pycbc that referenced this pull request May 21, 2024
* Use approximant parsing argument for make_bayestar

* SPAtmplt not in bayestar
spxiwh pushed a commit to spxiwh/pycbc that referenced this pull request May 28, 2024
* Use approximant parsing argument for make_bayestar

* SPAtmplt not in bayestar
spxiwh added a commit that referenced this pull request May 28, 2024
* Use approximant parsing argument for make_bayestar (#4712)

* Use approximant parsing argument for make_bayestar

* SPAtmplt not in bayestar

* Cherry pick [manually] 4683

* Bugfix: allow nothing to be given in --labels (#4681)

* Bugfix: allow nothing to be given in --labels

* Update bin/all_sky_search/pycbc_upload_single_event_to_gracedb

Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr>

---------

Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr>

* Bump version number

---------

Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org>
Co-authored-by: Tito Dal Canton <tito.dalcanton@ijclab.in2p3.fr>
GarethCabournDavies added a commit to GarethCabournDavies/pycbc that referenced this pull request Jul 24, 2024
* Use approximant parsing argument for make_bayestar

* SPAtmplt not in bayestar
@spxiwh spxiwh removed the v23_release_branch PRs applied to the v2.3.X release branch or to be cherry-picked if merging to master label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants