GWSignal: fix TEOBResumSDALI global sign (psi was displaced by pi/2) - #175
Open
oshaughnessy-junior wants to merge 1 commit into
Open
Conversation
oshaughnessy-junior
had a problem deploying
to
private-review-dispatch-rift-upstream
August 13, 2026 17:31 — with
GitHub Actions
Failure
gwsignal's TEOBResumSDALI GenerateTDModes output relates to the same
plugin's polarizations by
hp - i hc = - nu M G/(c^2 D) * sum_lm h_lm {-2}Y_lm(iota, 0)
Measured one (l,|m|) at a time via ModeArray, so that a single complex
scalar is exact and an azimuth convention cannot masquerade as a sign:
arg(c) = 180 deg with residual 0.0000 for m = 1, 2, 3 and 4 alike, and
|c| matching nu M G/(c^2 D) to six digits. A constant arg across m is a
global sign, not a phase convention.
hlmoft applied the magnitude but not the minus sign. This negates the
rescaling and changes nothing else. Other approximants are untouched:
RIFT's SEOBNRv5EHM mode sum already reproduces the gwsignal
polarizations exactly, every per-m coefficient 1.000 at 0.0 deg.
A global sign on h is exactly psi -> psi + pi/2, so this was not visible
as a bad fit. It displaced psi by a quarter turn in every RIFT
TEOBResumSDALI posterior in the eccentric-PE task force comparison while
leaving sky location, distance, masses, inclination and the peak
likelihood in agreement with bilby and dingo. Confirmed end to end:
before, the template RIFT labels psi_R matches the gwsignal/bilby
template at psi_R + 1.03 x pi/2; after, |dpsi| <= 0.10 x pi/2. Same
answer in H1, L1 and V1, at phi_ref = 0 and 0.3.
Deliberately NOT addressed here: TEOB's modes also do not want the
exp(i m phi_shift) that hlmoft applies, which leaves RIFT's reported
`phase` for this approximant offset by pi/2 from the polarization path.
That is a phase-convention difference rather than a sign error, it is
unchanged by this commit, and it is left for separate review since the
shift is load-bearing for every other approximant.
The regression test asserts the sign, not the convention. Fitting a free
complex coefficient c_m per azimuthal index, the templates are correct
iff c_m = s exp(i m delta) with s = +1; a common delta is only a
relabelling of phi_ref, but s = -1 is the bug. One m cannot separate
them -- -exp(i m delta) is reabsorbed into delta, which is why the
quadrupole alone never saw this -- so the test uses
arg(c_4) - 2 arg(c_2) == 0 if s = +1, == pi if s = -1
which is invariant under any choice of delta. It reads +178.7 deg on the
unpatched tree and passes after the fix, and it will keep passing if the
phase convention is revisited later.
The test skips where the TEOBResumSDALI plugin is absent. This branch
has no CI workflow, so run it manually where the plugin exists.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughnessy-junior
force-pushed
the
claude/teob-gwsignal-sign-fix-o4c
branch
from
August 13, 2026 19:14
5c35f58 to
f0a293a
Compare
oshaughnessy-junior
had a problem deploying
to
private-review-dispatch-rift-upstream
August 13, 2026 19:14 — with
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
RIFT/physics/GWSignal.pycorrects the magnitude of gwsignal'sTEOBResumSDALI modes but not their sign. RIFT's TEOBResumSDALI templates are
therefore
-1 xbilby's, which is exactlypsi -> psi + pi/2.One-line change: negate
distance_rescaling. Nothing else, and no otherapproximant is touched.
Why it was invisible
F+(psi+pi/2) = -F+(psi)andFx(psi+pi/2) = -Fx(psi), in every detector, forevery mode. So a global sign on
his exactly a quarter turn in psi andnothing else — sky location, distance, masses, inclination and peak
lnLallstill agree with bilby and dingo. The fit looks perfect.
Flagship case (M=120, chi_eff 0, ecc 0), injected
psi = pi/4 = 0.785:3pi/4 = 2.356. Across 18 comparisonspsi_rift = psi_bilby + pi/2wins forTEOBResumSDALI in 7 of 9 informative cases and never for SEOBNRv5EHM, which
is identity in all 8. dingo-vs-bilby is identity everywhere, TEOB included.
Root cause
The codes reach the same plugin through different doors:
GenerateFDWaveform-> polarizations--use-gwsignal) ->GenerateTDModes-> modes, then its ownharmonic sum
For this one plugin the modes are minus the polarization convention (and are
dimensionless). Restricting to one
(l,|m|)at a time viaModeArraymakeshp - i hc = c * sum_lm h_lm {-2}Y_lm(iota,0)exact for a single complexc,so an azimuth convention (mode-dependent,
e^{i m dphi}) cannot masquerade asa global sign:
arg(c) = 180 degfor every m, notarg(c) ∝ m. And8.50741e-22isnu M G/(c^2 D)for these parameters to six digits — precisely thedistance_rescalingalready in the code. The magnitude was patched; the signwas not.
RIFT is correct for SEOBNRv5EHM: fitting a free complex coefficient per
azimuthal index against the gwsignal polarizations gives
1.000at0.0 degfor all eight m. The
exp(i m pi/2)shift inhlmoftis right and necessarythere, and is left alone.
Verification
RIFT template held at
psi_R = pi/4; gwsignal/bilby template scanned overpsi_Twithphi_refmaximized over, signed overlap, all three detectors,run against the patched tree itself (not a monkeypatch):
psi_T - psi_RSame answer in H1, L1 and V1, at
phi_ref = 0and0.3.This also rules out a cross-polarization (
hx) sign error, the obvious firstguess: flipping only
hxis a reflection of(F+,Fx)rather than arotation, so no single psi could absorb it across three detectors and the sky
position would shift. It doesn't.
Deliberately not in this PR
TEOB's modes also do not want the
exp(i m phi_shift)thathlmoftapplies,which leaves RIFT's reported
phasefor this approximant offset bypi/2from the polarization path. That is a phase-convention difference rather than
a sign error; it exists today, is unchanged by this PR, and the shift is
load-bearing for every other approximant, so it is left for separate review.
Regression test
test/test_gwsignal_teob_mode_sign.pyasserts the sign, not theconvention.
Fitting a free complex coefficient
c_mper azimuthal index, the templatesare correct iff
c_m = s exp(i m delta)withs = +1: a commondeltaisonly a relabelling of
phi_ref, buts = -1is the psi bug. A single mcannot separate them —
-exp(i m delta)is reabsorbed intodelta, which isprecisely why the quadrupole alone never saw this. Two m values sharing one
deltado:invariant under any choice of
delta. It reads +178.7 deg on theunpatched tree and passes after the fix — and will keep passing if the phase
convention is revisited later, so it does not prejudge that question.
The test skips where the TEOBResumSDALI plugin is absent.
This branch has no CI workflow, so run it manually where the plugin exists:
Companion PR against
rift_O4d: oshaughnessy-junior#92🤖 Generated with Claude Code