Skip to content

Add NRSur7dq4 waveform approximant support#395

Merged
nihargupte-ph merged 1 commit into
hackathon-1from
nrsur7dq4
Jul 10, 2026
Merged

Add NRSur7dq4 waveform approximant support#395
nihargupte-ph merged 1 commit into
hackathon-1from
nrsur7dq4

Conversation

@nihargupte-ph

Copy link
Copy Markdown
Collaborator

Summary

Adds support for the NRSur7dq4 precessing surrogate in the synthetic-phase mode path (WaveformGenerator.generate_hplus_hcross_m), so it can be used with the phase treated as an extrinsic parameter.

Focused extraction of the NRSur7dq4 part of #258 — dropping SEOBNRv4EHM / SEOBNRv4HM and the unrelated changes.

Implementation

NRSur7dq4 (approximant 93) is a time-domain, precessing model whose modes are generated via SimInspiralChooseTDModes, exactly like SEOBNRv4PHM. So enabling it is a one-line change: add it to the SimInspiralChooseTDModes branch of generate_TD_modes_L0. The existing per-mode taper (taper_td_modes_in_place) then handles it like the other TD approximants.

We evaluated three tapering strategies against the direct SimInspiralFD polarizations, under phase shifts (the regime the method is used in), across NRSur7dq4's region of validity:

taper 10 Hz (high mass) 20 Hz
default per-mode max 1.1e-3, med 1.1e-4 max 1.1e-3, med 3.3e-4
common window (constant, from #258 co-author's suggestion) max 3.2e-3, med 2.7e-4 max 7.3e-3, med 6.2e-4
stepped-back conditioning (#258's intended branch) crashes / ~0.022 ~0.022

The default taper is as good or better in every case; the alternatives only help at fixed zero phase shift, which is not how generate_hplus_hcross_m is used. (Note: #258's own NRSur7dq4 tapering branch was dead code — a "NRsur7dq4" typo — so #258 also fell through to this default taper in practice.)

Testing

tests/gw/waveform_generator/test_wfg_m.py adds NRSur7dq4 to the mode-reconstruction test (phase-shifted sum of m-contributions vs. the directly-generated polarizations).

  • NRSur7dq4 has a hard validity region (mass ratio, spin, and — at fixed f_min — total mass). The prior is constrained to lie entirely inside validity (precessing, q >= 0.2, spins <= 0.8, chirp-mass floor 60 so f_min = 10 Hz is always reachable), so every sampled configuration generates without an out-of-domain error.
  • Tolerance (3e-3, 5e-4) for (max, median); observed max ~1.4e-3, median ~1.6e-4, comparable to SEOBNRv4PHM.
  • The test is skipped when the NRSur7dq4 surrogate data file is not installed ($LAL_DATA_PATH), via a guard that probes only raw lalsimulation, so a genuine regression still fails the test.
tests/gw/waveform_generator/test_wfg_m.py::test_generate_hplus_hcross_m[NRSur7dq4] PASSED

🤖 Generated with Claude Code

Enable the NRSur7dq4 precessing surrogate in the synthetic-phase mode path
(WaveformGenerator.generate_hplus_hcross_m), so it can be used with the phase
treated as an extrinsic parameter.

NRSur7dq4 (approximant 93) is a time-domain, precessing model whose modes are
generated via SimInspiralChooseTDModes, exactly like SEOBNRv4PHM. Enabling it
is therefore just adding it to the SimInspiralChooseTDModes branch of
generate_TD_modes_L0; the existing per-mode taper (taper_td_modes_in_place)
then handles it like the other TD approximants. Measured against the direct
SimInspiralFD polarizations across NRSur7dq4's region of validity, this default
taper gives a mode-reconstruction mismatch of ~1e-4 (tail up to ~1e-3) under
phase shifts -- as good as or better than a common-window or stepped-back
conditioning, which only help at fixed zero phase shift.

Focused extraction of the NRSur7dq4 support from PR #258 (dropping
SEOBNRv4EHM/SEOBNRv4HM and unrelated changes; #258's own NRSur7dq4 tapering
branch was dead code due to a "NRsur7dq4" typo, so it also used this default
taper in practice).

Test: tests/gw/waveform_generator/test_wfg_m.py adds NRSur7dq4 to the
mode-reconstruction test (reconstructed vs. direct polarizations), sampling a
precessing prior whose entire support lies inside the surrogate's validity
(chirp-mass floor of 60 so f_min = 10 Hz is always reachable). The test is
skipped when the NRSur7dq4 surrogate data file is not installed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nihargupte-ph nihargupte-ph requested a review from sclyne623 July 6, 2026 11:46
@AlexandreGoettel

Copy link
Copy Markdown
Collaborator

Just a note on the tests:

  1. there is no direct constraint on the chirp mass. The hard limit is in total_mass (from 60 to 400), which can be added as a constraint to the prior.
  2. the spins are only calibrated against NR up to 0.8, but the approximant was cleared to be ok to use up to 0.99
  3. the mass ratio is only calibrated down to 1 / 4, but here also the limit is lower: 1 / 6

@lorenzopompili00

lorenzopompili00 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Just a quick comment since I was looking at integrating different models (including NRSur7dq4) as part of the waveform generator refactor: I think the conditioning for NRSur7dq4 should follow what is done for SEOBNRv5PHM, rather than SEOBNRv4PHM, since it allows specifying the spins at a reference frequency different from the initial frequency.

In general, the logic should be based from here: https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lalsimulation/lib/LALSimInspiral.c#L4615. You can see that SEOBNRv4PHM sets spin_freq=LAL_SIM_INSPIRAL_SPINS_FLOW while NRSur7dq4 has spin_freq=LAL_SIM_INSPIRAL_SPINS_F_REF, which then determines the conditioning used in LAL https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lalsimulation/lib/LALSimInspiralGeneratorConditioning.c#L633-636

@nihargupte-ph

Copy link
Copy Markdown
Collaborator Author

Right, I think in the end this should be merged into or after your edits to the waveform generator @lorenzopompili00 . For now I just put up the PR for Alex to train a few NRSur networks. Maybe we merge this into your waveform generator branch, or possibly, you already have NRSur implemented in that branch?

@lorenzopompili00

Copy link
Copy Markdown
Collaborator

@nihargupte-ph I have a branch where I've been experimenting where I added NRSur support, but eventually I'd like to integrate those changes into Vincent's branch, which doesn't support it yet. I agree it makes sense to merge this once his branch reaches that stage. That said, if with this the goal with this is to get results that are consistent with Bilby, I'd suggest revisiting the conditioning. With the current code, I expect the DINGO and Bilby likelihoods won't exactly match for NRSur because of this (this might actually be a good test now that we are adding support for more waveforms!).

@sclyne623

Copy link
Copy Markdown
Collaborator

@nihargupte-ph @lorenzopompili00 I believe the conditioning here is already correct. The logic from https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lalsimulation/lib/LALSimInspiralGeneratorConditioning.c#L633-636 suggests NRSur7dq4 should follow the same conditioning routine as SEOBNRv4PHM according to the allow_zero_fmin_flag == LAL_SIM_INSPIRAL_ALLOW_ZERO_FMIN flag. In general tapering mode by mode and summing will not give the same result as a taper applied to the polarization directly, but the method here should work well enough for now and a more accurate tapering could be done at a later time.

@sclyne623 sclyne623 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me!

@lorenzopompili00

Copy link
Copy Markdown
Collaborator

@nihargupte-ph @lorenzopompili00 I believe the conditioning here is already correct. The logic from https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lalsimulation/lib/LALSimInspiralGeneratorConditioning.c#L633-636 suggests NRSur7dq4 should follow the same conditioning routine as SEOBNRv4PHM according to the allow_zero_fmin_flag == LAL_SIM_INSPIRAL_ALLOW_ZERO_FMIN flag. In general tapering mode by mode and summing will not give the same result as a taper applied to the polarization directly, but the method here should work well enough for now and a more accurate tapering could be done at a later time.

I agree with @sclyne623! I didn't realize there was the extra LAL_SIM_INSPIRAL_ALLOW_ZERO_FMIN clause in that if statement

@nihargupte-ph

nihargupte-ph commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Ok I then I will merge into hackathon-1, we can update this with the waveform generator refactor.

@nihargupte-ph nihargupte-ph merged commit 2f50ee8 into hackathon-1 Jul 10, 2026
1 check passed
@nihargupte-ph nihargupte-ph deleted the nrsur7dq4 branch July 10, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants