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

ENH: add Planck15_LAL cosmology #829

Merged
merged 4 commits into from
Nov 6, 2024
Merged

Conversation

mj-will
Copy link
Collaborator

@mj-will mj-will commented Oct 9, 2024

Add the Planck15_LAL cosmology as defined in: https://dcc.ligo.org/DocDB/0167/T2000185/005/LVC_symbol_convention.pdf

Note: there are additional commits in this PR since it requires the PR listed below. It should be rebased before being reviewed.

Requires: #828

@mj-will mj-will added the enhancement New feature or request label Oct 9, 2024
@mj-will mj-will changed the title Planck15 lal cosmo ENH: add Planck15_LAL cosmology Oct 9, 2024
@mj-will mj-will added this to the 2.4.0 milestone Oct 31, 2024
@mj-will mj-will force-pushed the planck15-lal-cosmo branch from 4b611f2 to 97fa8ce Compare November 4, 2024 09:26
@mj-will mj-will marked this pull request as ready for review November 4, 2024 09:26
@cplb
Copy link
Collaborator

cplb commented Nov 4, 2024

I can confirm that these are the conventional numbers dating back to https://arxiv.org/abs/1602.03840

@adivijaykumar
Copy link
Collaborator

Code-wise this looks good to me! Should we consider adding a test comparing luminosity distances calculated by lal and bilby at some fiducial redshift value(s)? I am not sure the python lal interface allow for this though.

@mj-will mj-will force-pushed the planck15-lal-cosmo branch from c5a450f to 759f48f Compare November 6, 2024 10:37
@mj-will
Copy link
Collaborator Author

mj-will commented Nov 6, 2024

Code-wise this looks good to me! Should we consider adding a test comparing luminosity distances calculated by lal and bilby at some fiducial redshift value(s)? I am not sure the python lal interface allow for this though.

I've added a test for this, for now it uses a manual version of the LAL cosmology but once changes from https://git.ligo.org/lscsoft/lalsuite/-/merge_requests/2368 at in a release, we can switch to that.

Copy link
Collaborator

@adivijaykumar adivijaykumar left a comment

Choose a reason for hiding this comment

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

Thanks @mj-will! Everything looks good to me. Started #852 to remind ourselves to change the tests in the future.

@mj-will mj-will merged commit 16025c8 into bilby-dev:main Nov 6, 2024
10 checks passed
JasperMartins pushed a commit to JasperMartins/bilby that referenced this pull request Nov 12, 2024
* ENH: add Planck15_LAL cosmology

* MAINT: add Planck15_LAL to known cosmologies

* DOC: update doc-string

Co-authored-by: Colm Talbot <[email protected]>

* TST: add test for Planck15 LAL cosmology

---------

Co-authored-by: Colm Talbot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants