Skip to content

Add pointwise optimized mismatch calculation#191

Open
keefemitman wants to merge 6 commits into
sxs-collaboration:mainfrom
keefemitman:main
Open

Add pointwise optimized mismatch calculation#191
keefemitman wants to merge 6 commits into
sxs-collaboration:mainfrom
keefemitman:main

Conversation

@keefemitman

@keefemitman keefemitman commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

This adds the usual LVK mismatch calculation of sampling a series of points on the two sphere, then for each point optimizing over time, binary phase, and polarization to minimize the residual, and returning the maximal mismatch obtained across all of the sky points. @moble probably worth you looking at, but @duetosymmetry probably also of interested to you.


📚 Documentation preview 📚: https://sxs--191.org.readthedocs.build/en/191/

Comment thread sxs/waveforms/norms.py Outdated
Comment thread sxs/waveforms/norms.py
Comment thread sxs/waveforms/norms.py Outdated
Comment thread sxs/waveforms/norms.py Outdated
Comment on lines +375 to +377
for mode in include_modes:
wa_sliced_data[:, wa_sliced.index(*mode)] *= 0
wb_sliced_data[:, wb_sliced.index(*mode)] *= 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this the inverse of the mask you want? This is setting all modes in include_modes to vanish.

Comment thread sxs/waveforms/norms.py Outdated
raise ValueError(f"(t1, t2)=({t1}, {t2}) is not contained in wb.t.")
if wa.spin_weight != wb.spin_weight:
raise ValueError(
f"Spin weights must match, but got wa.spin_weight={wa.spin_weight} " f"and wb.spin_weight={wb.spin_weight}."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
f"Spin weights must match, but got wa.spin_weight={wa.spin_weight} " f"and wb.spin_weight={wb.spin_weight}."
f"Spin weights must match, but got {wa.spin_weight=} and {wb.spin_weight=}."

@codecov-commenter

codecov-commenter commented Apr 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 8.74317% with 167 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.18%. Comparing base (43fa055) to head (d8c7a6f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
sxs/waveforms/norms.py 8.74% 167 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #191      +/-   ##
==========================================
- Coverage   43.19%   42.18%   -1.02%     
==========================================
  Files          85       85              
  Lines        6473     6650     +177     
  Branches     1066     1094      +28     
==========================================
+ Hits         2796     2805       +9     
- Misses       3423     3592     +169     
+ Partials      254      253       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread sxs/waveforms/norms.py
Comment thread sxs/waveforms/norms.py
wb_sliced_data = wb_sliced.data
for L in range(wa.ell_min, wa.ell_max + 1):
for M in range(-L, L + 1):
if not (L,M) in include_modes:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think stylistically we would prefer (L,M) not in include_modes, even though the result is the same

Comment thread sxs/waveforms/norms.py
"A `modes` argument was provided, but the input `wa` "
"and `wb` only have one dimension."
)
raise ValueError("A `modes` argument was provided, but the input `wa` " "and `wb` only have one dimension.")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This whitespace change looks funny...

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