Skip to content

Fix calmarg forwarding of ChooseFDModes frame options - #192

Merged
oshaughn merged 1 commit into
oshaughn:rift_O4cfrom
oshaughnessy-junior:fix/pr183-calmarg-fd-l-frame
Sep 11, 2026
Merged

oshaughn merged 1 commit into
oshaughn:rift_O4cfrom
oshaughnessy-junior:fix/pr183-calmarg-fd-l-frame

Conversation

@oshaughnessy-junior

Copy link
Copy Markdown

Summary

  • Expand calmarg waveform options into the named hlmoft arguments.
  • Apply the correction to the original BBH, current BBH, and eccentric source functions.
  • Add a regression test proving fd_L_frame and sibling options reach hlmoft without mutating the caller dictionary.

Why

PR 183 corrected the ChooseFDModes J-to-L rotation, but calibration reweighting supplied its options as one extra_waveform_kwargs catch-all keyword. hlmoft never read that nested bundle, so fd_L_frame remained false on the calmarg source path. XPHM calmarg could therefore use unrotated J-frame modes while ILE used L-frame modes.

Adversarial validation

  • Actual XPHM calmarg q=4 stress path received fd_L_frame, no_condition, and fd_alignment_postevent_time as named arguments; no nested extra_waveform_kwargs remained.
  • Rotated versus deliberately unrotated plus strain changed by 0.644635, demonstrating that the test is sensitive to the missing forwarding.
  • PR 183 waveform comparisons passed: fiducial overlap 0.999175, q=4 edge-on 0.996263, near-aligned 0.997735.
  • Maximum intrinsic-mode change under a psi shift was 0 in all three cases.
  • The regression test body passed in the supported RIFT Python 3.13 environment; both files compile and git diff --check passes.

Environment caveat

The local LALSimulation 6.1.0 provides XPHM and XO4a but not XPNR. A pre-existing grouped optional import in RIFT consequently masks all three identifiers. For the waveform validation only, the installed XPHM identifier was restored at runtime so the PR 183 route could be exercised. No workaround for that separate compatibility defect is included here.

@oshaughnessy-junior
oshaughnessy-junior deployed to private-review-dispatch-rift-upstream September 11, 2026 15:00 — with GitHub Actions Active
@oshaughn
oshaughn merged commit 71e5b50 into oshaughn:rift_O4c Sep 11, 2026
1 check passed
@oshaughn
oshaughn deployed to private-review-dispatch-rift-upstream September 11, 2026 15:17 — with GitHub Actions Active
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.

2 participants