Skip to content

Adding support for SEOBNRv5EHM#332

Open
nihargupte-ph wants to merge 54 commits into
mainfrom
seobnrv5ehm
Open

Adding support for SEOBNRv5EHM#332
nihargupte-ph wants to merge 54 commits into
mainfrom
seobnrv5ehm

Conversation

@nihargupte-ph

Copy link
Copy Markdown
Collaborator

This PR adds support for SEOBNRv5EHM through the new waveform interface. Note that unlike SEOBNRv5PHM and SEOBNRv5HM, SEOBNRv5EHM uses different conditioning routines. In particular, it doesn't use the stepping back of the starting frequency since the reference frequency is equal to the starting freqeuncy. This is why different routines are use when doing the summation in the m modes.

stephengreen and others added 30 commits June 2, 2025 21:52
Monkey-hack of bilby_pipe.create_injections.py.
Ensure everything but the inference parameters are stripped from theta. For samples coming from Result, existing values for log_prob were being added to new ones.
This is needed for models with DeltaFunction ra priors.
Adapted from Bilby. Also added parameter names to default priors, so that they pick up latex labels for plotting.
To use, set plot-pp = true in INI file.
Set dingo-injections = true in INI file. Noise is still generated using bilby_pipe.
Co-authored-by: Nihar Gupte <40392612+nihargupte-ph@users.noreply.github.com>
…g to change it via args.injection-waveform-arguments
Adding BLAS and MKL threading options so that the scipy/numpy svd function doesn't segfault
Adding BLAS and MKL threading options so that the scipy/numpy svd function doesn't segfault
@lorenzopompili00

Copy link
Copy Markdown
Collaborator

Looks good! I left a few small comments.

On a longer term, I would love for this code not to have any line like

if self.approximant_str == NAME

but to look up relevant information from the gwsignal metadata, using gwsignal_get_waveform_generator, so that the code works with any eccentric model, and one does not need to keep updating it when a new one is added. One can (in principle) check if the waveform is, e.g., eccentric, with

wf_gen = gwsignal_get_waveform_generator(self.approximant_str)
if wf_gen.metadata["type"] == "eccentric":

and wf_gen.metadata["f_ref_spin"], wf_gen.metadata["f_ref_ecc"] specify if spins and/or eccentricity can be defined at an arbitrary f_ref, which could be used to automatically determine which conditioning routine is appropriate.

Still, I realize things are not too polished on the gwsignal side so it's probably not worth thinking now about these kind of changes (TEOBResumS has a few more info, but SEOBNRv5EHM is still lacking some of these); I'm leaving this comment anyway to keep this in mind, one can update this in the future.

nihargupte-ph and others added 20 commits November 13, 2025 11:56
- Consolidate TEOB mode generation into generate_TD_modes_L0 with
  return_lal parameter (True by default, False for TEOB gwpy pipeline).
  Fixes incorrect negative distance rescaling sign for TEOB.
- Fix typo in TEOB branch condition: "TEOBREsumSDALI" -> "TEOBResumSDALI"
- Remove dead code from wfg_utils.py: lalseries_to_gwpy_timeseries_in_place,
  gwpyseries_to_lalseries_in_place, and the entire Stage 1/2 TEOB
  conditioning chain (get_conditioning_params_for_TEOB,
  condition_td_modes_for_TEOB_in_place, _high_pass_complex,
  _condition_stage1_complex, _condition_stage2_complex) which was
  superseded by the sigmoid taper approach.
- Clean up unused imports (butter, sosfiltfilt, astropy.units).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use parameters["phase"] instead of self.spin_conversion_phase for TEOB
epoch computation and spherical harmonics, consistent with SEOBNRv5EHM.
Pass deferred_timeshift_data in test for correct phase-dependent epoch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove leftover debug raise that broke CI when EOBRun_module isn't
installed. Restructure approximant detection so pyseobnr and
EOBRun_module are checked independently and contribute their
approximants additively.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The debug branch was enabled and tried to savefig to a hardcoded
local path that does not exist on CI runners, causing the CI failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace dingo's reimplementation of the sigmoid taper
(_compute_sigmoid_taper_params, _compute_sigmoid_window) with a
get_tapering_window_for_real_array helper that calls
cond.taper_gwpy_timeseries on a copy and recovers the window via
|tapered|/|original|, mirroring the LAL-side
get_tapering_window_for_complex_time_series trick. TEOB mode-resum
output is bit-identical to the previous implementation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PESummary's standard_names dictionary maps spin_1z/spin_2z but not
chi_1/chi_2, so without this rename the spin conversion path is skipped
and a_1, chi_eff, chi_p, final_spin etc. collapse to zero in the
generated webpages. Apply the rename in get_pesummary_samples and the
matching pesummary_prior so derived spin quantities are computed from
the actual posterior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

3 participants