Merge imrphenomxp_nrtidalv2 branch

Description

This MR adds

• additional precessing and final spin versions to existing PhenomX code, through flags that can be passed by the user. The default behaviour of PhenomX models is unchanged
• two new tidal approximants, IMRPhenomXAS_NRTidalv2 and IMRPhenomXP_NRTidalv2, where the NRTidal_v2 extension has been ported to the PhenomX code baseline.
• a check on the sign of tidal deformabilities which solves an existing problem in PhenomPv2_NRTidal models. The patch is applied in lalsimulation/lib/LALSimNRTunedTides.c and only modifies the behaviour of current models when negative lambdas are passed, resulting in a hard XLAL error message.

API Changes and Justification

Backwards Compatible Changes

• This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions
• This change adds new classes/functions/structs/types to a public C header file or Python module

Backwards Incompatible Changes

• This change modifies an existing class/function/struct/type definition in a public C header file or Python module
• This change removes an existing class/function/struct/type from a public C header file or Python module

If any of the Backwards Incompatible check boxes are ticked please provide a justification why this change is necessary and why it needs to be done in a backwards incompatible way.

Review Status

Review complete: review statement. Review chair: @maria.haney

added 1 commit

• 6236b2b1 - add author to lalsimulation

Compare with previous version

added lalsimulation label

added 1 commit

• 1d29bad7 - delete redundant variables in XPHM.c

Compare with previous version

marked the checklist item This change does not modify any class/function/struct/type definitions as completed

marked the checklist item This change adds new classes/functions/structs/types as completed

marked the checklist item This change adds new classes/functions/structs/types as incomplete

marked the checklist item This change adds new classes/functions/structs/types as completed

added apiminor label

added 1 commit

• 35d3c982 - slightly increase tolerance of unit test for XPHM

Compare with previous version

added 1 commit

• 6498fe7c - update authors

Compare with previous version

added 1 commit

• 58a4be5f - add warnings and patch for high mass cases

Compare with previous version

added 1 commit

• 65fe4521 - fix typo in previous commit

Compare with previous version

changed the description

added 1 commit

• fa949b8e - cosmetic change

Compare with previous version

added 174 commits

• fa949b8e...a7c2b377 - 98 commits from branch lscsoft:master
• 063731ae - Initial implementation of NRTidal in PhenomXP (currently hard-coded so that...
• 0562814f - add extra tests
• f3f66a58 - adopt same convention as TPHM to switch on different final spin (FS=4)
• f4415d41 - reorder approximant names
• 13bd8716 - fix brackets in commit a7784e48
• 586a185d - graceful failures
• 2a4a019f - remove strict error raising
• 74a53dc0 - fix aligned spin limit
• b81097f1 - delete test code
• 32d07209 - Setting the NRTidalv2 coefficients with a single function
• 1c9670cc - Some small cleanup in LALSimIMRPhenomX_precession.c
• 916afc2b - clean up test routines and fix failure
• 442c1c0e - test XP failures again
• e29903b8 - complete previous commit to test_phenomX
• 27416492 - update unit test
• f14683a9 - default to NNLO angles when in-plane spins very small
• 6d4eed7a - correct XLAL function for Euler angles
• 49c9ee18 - correct implementation of AS limit
• 033359b6 - implement fallback to MSA+NNLO
• c6f93dc5 - free PN series (fix to previous commit)
• 3afe61cf - delete redundant lines to deallocate memory
• 7dd0ff4f - add documentation
• b126626f - add documentation
• 15f721f5 - fix bug in IMRPhenomXPHM_hplushcros_from_modes for new prec versions
• b16b67ab - add missing line
• 04fddc74 - add missing brackets
• d2a6fcdc - initialise modes for correct output of IMRPhenomX_GetandSetModes
• e9310b3c - code changes to make new versions compatible w XPHMFrequencySequenceOneMode
• f1c97226 - update XLAL function returning Euler angles
• a0084c31 - set integration pars to avoid discontinuous timings
• d6900204 - LALSimNRTunedTides.c: Adding a Doxygen comment block for...
• aebc4347 - test_phenomX.py: Small fixes to the documentation
• e51b0731 - LALSimIMRPhenomX_precession.c: Turning descriptions into Doxygen descriptions...
• 51e35ab7 - LALSimIMRPhenomX_precession.c: Fixing the LaTeX in function descriptions for Doxygen
• 05cc9d10 - update unit tests (after decreasing integration step)
• 317d5039 - LALSimIMRPhenomXUtilities.c: Doxygenation of descriptions
• 0697b703 - LALSimIMRPhenomXUtilities.c: Fixing a typo
• afb10c7d - LALSimIMRPhenomXUtilities.c: Fixing another typo
• 2f1ca998 - catch fallback in xlal euler angles function
• 934399cd - change coarse factors, add option to evolve on single grid
• 72f7dcf8 - disable hard-coded coarse factor for testing
• d4296665 - trigger error in MRD angles when appropriate
• 73fe23ea - add support for XLALSimIMRPhenomXPHMOneMode
• 7e39a45f - change call to betaMRD_coeff
• 619d4cde - fix typo in quadparams
• 3b07cfd8 - Updating PhenomX documentation to include the NRTidalv2 extension and...
• 0f3a8e2c - LALSimIMRPhenomX.c: Fixing the use of the @review tag
• 252670f4 - test_phenomX.py: Updating BBH SpinTaylor regression test data after the...
• 8876473a - fix issue with unequal spacings in farray
• 757c748d - add comments to functions
• 5b76ca26 - fix bug in PV310
• 8646cb23 - fix time shift
• 6e35f23e - updated unit tests
• 7e82a4b2 - disable forced alignment in XAS_NRTidalv2
• 14f8552d - add error messages for tidal deformabilities
• ea582c99 - make names of functions more self-explanatory and add comments
• be747b0c - add documentation
• 17c8d7ba - re-formulate addition of tidal phase at fRef
• 28c6b9c7 - fix gsl failures at very high masses
• 82f6c7fd - add fMax code changes to hopefully fix rebasing
• d5fe32a1 - update unit tests
• 89e006fb - fix brackets messed up by rebasing
• e4e0a8d1 - update unit tests
• 7342532d - set same tolerance as in other HM models for ST approx
• 0e8f93a8 - improve gsl error checks
• 50873a8e - add error message fixing Pv2_NRTidal bug for negative lambdas
• b0744d53 - fix bug in gamma interpolation introduced just before rebasing
• a7194871 - fix typo introduced in last commit and update unit tests again
• 7a80257b - rename some internal functions and add documentation
• c0f00b63 - enforce default PN orders when not specificed by the user
• 1373fd61 - add author to lalsimulation
• 41f7a971 - delete redundant variables in XPHM.c
• be1cf597 - slightly increase tolerance of unit test for XPHM
• 82dbf6f1 - add warnings and patch for high mass cases
• 23c38cb8 - fix typo in previous commit
• c8ed5585 - cosmetic change

Compare with previous version

@geraint.pratten @vijay.varma Review is complete and signed off. (Write-up in review statement pending.)

added 1 commit

• 38cba7f4 - fix enum problem

Compare with previous version

@duncanmmacleod: I'm seeing a lot permission denied errors in the platform-tests stage of the pipeline. We've noticed that other developers hit a similar problem yesterday. How can this be sorted out? Thanks in advance!

added 1 commit

• 797e1fd1 - fix for SEOB: add SEOBNRv5_ROM to test_enum_compatibility, as it was omitted

Compare with previous version

added 1 commit

• 5c8bd103 - update .mailmap

Compare with previous version

added 218 commits

• 5c8bd103...4be3f28d - 161 commits from branch lscsoft:master
• d6569f16 - Initial implementation of NRTidal in PhenomXP (currently hard-coded so that...
• 3cb24228 - graceful failures
• fd4cccf0 - fix aligned spin limit
• d315f788 - default to NNLO angles when in-plane spins very small
• eedd0935 - correct XLAL function for Euler angles
• a0c25dfe - correct implementation of AS limit
• e2db7349 - implement fallback to MSA+NNLO
• 17652baa - free PN series (fix to previous commit)
• 4eaeed39 - delete redundant lines to deallocate memory
• 47707b76 - add documentation
• ca9018f9 - LALSimNRTunedTides.c: Adding a Doxygen comment block for...
• f5d6496a - test_phenomX.py: Small fixes to the documentation
• 7f9e58e9 - LALSimIMRPhenomX_precession.c: Turning descriptions into Doxygen descriptions...
• f92d949b - LALSimIMRPhenomX_precession.c: Fixing the LaTeX in function descriptions for Doxygen
• 5c01f744 - update unit tests (after decreasing integration step)
• 2b16fcf5 - LALSimIMRPhenomXUtilities.c: Doxygenation of descriptions
• 4982c1b6 - LALSimIMRPhenomXUtilities.c: Fixing a typo
• 7ed8b993 - LALSimIMRPhenomXUtilities.c: Fixing another typo
• 6d8e7d82 - catch fallback in xlal euler angles function
• 6d9238d4 - change coarse factors, add option to evolve on single grid
• 9dce4280 - disable hard-coded coarse factor for testing
• f7f3c116 - trigger error in MRD angles when appropriate
• d2bd500a - add support for XLALSimIMRPhenomXPHMOneMode
• 27dab2ba - change call to betaMRD_coeff
• 73e0b20a - fix typo in quadparams
• e0b94d80 - Updating PhenomX documentation to include the NRTidalv2 extension and...
• 08ac76a3 - LALSimIMRPhenomX.c: Fixing the use of the @review tag
• 201a7df7 - test_phenomX.py: Updating BBH SpinTaylor regression test data after the...
• 2f4a5f8b - fix issue with unequal spacings in farray
• 3009cce4 - add comments to functions
• a0267451 - fix bug in PV310
• a8c3265b - fix time shift
• 946ca41a - updated unit tests
• 44d2f275 - disable forced alignment in XAS_NRTidalv2
• b1ddf761 - add error messages for tidal deformabilities
• 5ab62372 - make names of functions more self-explanatory and add comments
• 668b2358 - add documentation
• 2f9478c2 - re-formulate addition of tidal phase at fRef
• a833f3dd - fix gsl failures at very high masses
• 51ddd5d7 - add fMax code changes to hopefully fix rebasing
• e71b08d1 - update unit tests
• febdcfaf - fix brackets messed up by rebasing
• 65cf1b89 - update unit tests
• 64a68701 - set same tolerance as in other HM models for ST approx
• 79de7932 - improve gsl error checks
• 42c7f4df - add error message fixing Pv2_NRTidal bug for negative lambdas
• 14f08492 - fix bug in gamma interpolation introduced just before rebasing
• c87380c6 - fix typo introduced in last commit and update unit tests again
• 145217eb - rename some internal functions and add documentation
• 82f87188 - enforce default PN orders when not specificed by the user
• ac4ca681 - add author to lalsimulation
• c46d0350 - delete redundant variables in XPHM.c
• c710d747 - slightly increase tolerance of unit test for XPHM
• 18b32208 - add warnings and patch for high mass cases
• 35a447e0 - fix typo in previous commit
• 5c64e4d6 - cosmetic change
• 6300243c - update .mailmap

Compare with previous version

added 1 commit

• 6a32ad17 - fix white space

Compare with previous version

resolved all threads

@maria.haney @vijay.varma @geraint.pratten: I rebased the code this afternoon to incorporate all the latest changes in lalsim, including the new XHM amplitudes. Do you have further requests/comments? @duncanmmacleod @adam-mercer @karl-wette: let me know if I have your approval re. the changes to .mailmap and AUTHORS. Thanks a lot in advance.

This is still marked as a draft, can you mark it ready if it's ready for approval?

changed the description

@geraint.pratten @vijay.varma Review statement(s) complete. I've tentatively approved this, but still needs your confirmation. This also needs to be marked 'ready' by @marta.colleoni. Then we'll have to coordinate between this and !2122 (merged) ...

approved this merge request

approved this merge request

I think this is ready for merger once marked ready.
In terms of coordination, maybe @frank-ohme can give an estimate of when !2122 (merged) may be fully ready for merger, as we don't want to delay this merger too much.

@cecilio.garcia-quiros is working on resolving conflicts in !2122 (merged) and said he would try to conclude this work before lunchtime in Europe today.

I managed to make it compile, but the unit test for XO4a is failing. I need to leave now and I will be out until night I think. Apologies about that.

https://git.ligo.org/waveforms/reviews/lalsuite/-/tree/phenomXO4-review-tomaster

Thanks both (and the XO4a team). At this stage, given that !2122 (merged) still needs conflict resolution, we will go ahead and merge this MR.

marked this merge request as ready

changed title from Draft: merge imrphenomxp_nrtidalv2 branch to Merge imrphenomxp_nrtidalv2 branch

approved this merge request

resolved all threads

mentioned in commit e903893e

merged

mentioned in merge request !2122 (merged)

mentioned in issue #698 (closed)