Skip to content

lalsimutils: phi12, chi_p_vec, vectorized in-plane spin coordinates for CIP - #203

Open
oshaughnessy-junior wants to merge 3 commits into
oshaughn:rift_O4cfrom
oshaughnessy-junior:claude/ring-coordinate-cip
Open

oshaughnessy-junior wants to merge 3 commits into
oshaughn:rift_O4cfrom
oshaughnessy-junior:claude/ring-coordinate-cip

Conversation

@oshaughnessy-junior

@oshaughnessy-junior oshaughnessy-junior commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Adds in-plane spin coordinates for CIP fits.

  • extract_param('phi12') (listed in valid_params, not implemented) and extract_param('chi_p_vec'), the vector-sum analogue of chi_p with its weights.
  • The spherical branch of convert_waveform_coordinates builds chi1_perp, chi2_perp, phi12, SOverM2_perp, DeltaOverM2_perp and chi_p_vec vectorized. It accepts CIP's object arrays and keeps the enforce_kerr rule. phi12 is 0 when an in-plane spin vanishes. chi_p_vec is not in valid_params.

No defaults change.

Why: on low-mass O4b events, fitting chi_p_vec moves three chi1_perp deficit events from 0.70/0.70/0.84 of bilby to 0.87/0.88/1.01. It also widens healthy high-mass controls by up to +0.15, so it is not a default. Evidence: RIFT_roboto_paper analyses/transverse_convergence/RESULTS_ring_coordinate_2026-09-30.md.

Tests (ldas-grid, IGWN python, numpy):

  • test_ring_coordinates.py, now in the cip-startup and GitLab jobs: 7 passed.
  • CIP-startup, EOS, sky-rotation, time-marginalization tests: 40 passed. Existing coordinate sets are bit-identical to the base, with and without enforce_kerr.
  • CIP with the default sampler and chi_p_vec/phi12: exit 0.

🤖 Generated with Claude Code

…nates

extract_param gains phi12 (listed in valid_params but not implemented)
and chi_p_vec, the vector-sum analogue of chi_p (same A1, A2 weights).
The spherical branch of convert_waveform_coordinates now builds
chi1_perp, chi2_perp, phi12, SOverM2_perp, DeltaOverM2_perp and chi_p_vec
vectorized. Test: vectorized path agrees with extract_param to 1e-9.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in-plane spin

Review of #203 found that CIP's default sampler (adaptive_cartesian) passes an
object array of python floats, and the vectorized ring block's np.sqrt raised
TypeError, so CIP exited 1 with these fit coordinates. The block now casts to
float. phi12 returns 0 in both paths when either in-plane spin vanishes (the
two paths disagreed there), and phi12 joins periodic_params. Tests: phi12 at
pi/3 in both paths (catches a sign flip in both), object-array input, zero
in-plane spin. test_ring_coordinates.py is added to the cip-startup workflow
and .gitlab-ci.yml, which list their tests explicitly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oshaughnessy-junior
oshaughnessy-junior deployed to private-review-dispatch-rift-upstream September 30, 2026 22:39 — with GitHub Actions Active
…arams

From review of the O4d port (#377), which applies here too:
- The vectorized in-plane block can end convert_waveform_coordinates before the
  per-row fallthrough, whose enforce_kerr rule then never ran. The block now
  applies the same rule (row set to -inf if chi1 or chi2 > 1). Sets that do not
  use the in-plane names are bit-identical to the base, with and without
  enforce_kerr.
- chi_p_vec is removed from valid_params: grid readers call assign_param on
  every listed column, and chi_p_vec is derived only. CIP does not need it there.
- phi12 cannot return exactly 2 pi.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oshaughnessy-junior
oshaughnessy-junior deployed to private-review-dispatch-rift-upstream September 30, 2026 22:45 — with GitHub Actions Active
@oshaughnessy-junior
oshaughnessy-junior marked this pull request as ready for review September 30, 2026 22:59
@oshaughnessy-junior
oshaughnessy-junior deployed to private-review-dispatch-rift-upstream September 30, 2026 22:59 — with GitHub Actions Active

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Independent automated review completed at the recorded exact commit. Detailed findings were withheld from public output by the private-context egress policy and require private human declassification.

This branch was successfully deployed

1 active deployment
private-review-dispatch-rift-upstream — 4ceb22c8 Deployed Sep 30, 2026 by oshaughnessy-junior via Dispatch exact upstream RIFT PR generation #123
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.

1 participant