Skip to content

docs: fix matched filter SNR docstrings (closes #731)#1069

Open
christianescamilla15-cell wants to merge 1 commit intobilby-dev:mainfrom
christianescamilla15-cell:fix/matched-filter-snr-docstring
Open

docs: fix matched filter SNR docstrings (closes #731)#1069
christianescamilla15-cell wants to merge 1 commit intobilby-dev:mainfrom
christianescamilla15-cell:fix/matched-filter-snr-docstring

Conversation

@christianescamilla15-cell
Copy link
Copy Markdown

Summary

Fixes #731 — the docstrings for matched_filter_snr and optimal_snr_squared in bilby/gw/utils.py were inaccurate.

Changes

matched_filter_snr() (line 158)

The docstring stated the function returned the matched filter SNR squared, but it actually returns the complex matched filter SNR (not squared). The function computes:

rho_mf = <signal | strain> / sqrt(<signal | signal>)
       = <s|h> / sqrt(optimal_snr_squared)

This is the standard complex matched filter SNR. Dividing by sqrt(<s|s>) (the optimal SNR) gives the matched filter SNR directly, not its square.

Before:

float: The matched filter signal to noise ratio squared

After:

complex: The complex matched filter signal to noise ratio

optimal_snr_squared() (line 187)

Following @ColmTalbot's suggestion in the issue discussion, updated the docstring to be more precise — calling it "the square of the optimal matched filter SNR" rather than just "matched filter SNR squared".

Before:

float: The matched filter signal to noise ratio squared

After:

float: The square of the optimal matched filter signal to noise ratio

Test plan

  • No code behavior changed — only docstrings
  • CHANGELOG.md updated under [Unreleased]
  • Verified function behavior matches corrected documentation

References

  • Maggiore, "Gravitational Waves Vol 1", Chapter 7
  • Finn 1992 (PRD 46, 5236) — matched filtering for gravitational wave detection

The docstring for matched_filter_snr() incorrectly stated it returned
the SNR squared, but the function actually returns the complex matched
filter SNR (not squared):

    rho_mf = <s|h> / sqrt(<s|s>)

This is the standard complex matched filter SNR, where <.|.> is the
noise-weighted inner product. The denominator sqrt(<s|s>) is the
optimal SNR, so dividing by it yields the matched filter SNR directly.

Additionally, updated the optimal_snr_squared() docstring to be more
precise by calling it "the square of the optimal matched filter SNR"
as suggested by @ColmTalbot in the issue discussion.

References:
- Maggiore, "Gravitational Waves Vol 1", Chapter 7
- Finn 1992 (PRD 46, 5236)
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.

Incorrect documentation for matched filter SNR ?

1 participant