Fix intrinsic and raw coherence

[matteobachetti]

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.48%. Comparing base (79b8f9b) to head (3e85b44).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #965      +/-   ##
==========================================
- Coverage   96.21%   95.48%   -0.73%     
==========================================
  Files          48       48              
  Lines        9949    10034      +85     
==========================================
+ Hits         9572     9581       +9     
- Misses        377      453      +76     

☔ 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.

[Copilot]

Pull request overview

This PR fixes the calculation of raw and intrinsic coherence by properly handling negative bias terms and adding support for uncertainty calculations. The changes implement corrections based on Vaughan & Nowak 1997 and Ingram 2019, addressing cases where the cross-spectrum magnitude squared is smaller than the bias term.

Changes:

• Added new intrinsic_coherence function in fourier.py with uncertainty calculation support
• Modified raw_coherence to handle negative numerators by setting coherence to 0 instead of ignoring bias
• Added _apply_low_lim_to_coherence_uncertainty helper function to ensure valid uncertainty values
• Updated AveragedCrossspectrum.coherence to use new uncertainty calculation from raw_coherence
• Added AveragedCrossspectrum.intrinsic_coherence method for intrinsic coherence calculations

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

File	Description
stingray/fourier.py	Core implementation of coherence fixes, new intrinsic_coherence function, uncertainty handling, and debug print statement accidentally left in error_on_averaged_cross_spectrum
stingray/crossspectrum.py	Updated coherence methods to use new functions with uncertainty support, added intrinsic_coherence method, but incorrectly passes self.n instead of self.m for n_ave parameter
stingray/tests/test_fourier.py	Updated test to verify coherence is 0 when bias term exceeds cross-spectrum magnitude, removed warning check

Comments suppressed due to low confidence (1)

stingray/fourier.py:809

• Incomplete return value documentation. When return_uncertainty is True, the function returns a tuple (coherence, uncertainty), but the Returns section only documents the coherence value. The documentation should specify that when return_uncertainty=True, a tuple is returned.

    Returns
    -------
    coherence : float `np.array`
        The raw coherence values at all frequencies.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[eleonorav89]

I have not fount errors in the formulas and I agree with the changes and improvements. Not an expent in dealing with such uncertainties, therefore I trust you. As I already mentioned in the comments, I think it should be useful to have the intrinsic coherence in the documentation with a plot as well indicating its correctness. One important thing I got is that people were confunsing the raw and the intrinsic when using the "old" coherence function. Thank you Matteo for your work.

[eleonorav89]

For what I got from the users, people prefer to use the intrinsic coherence with respect to the raw one. Should we swap them in the documentation or add a few line for the intrinsic one?

[matteobachetti]

Rather than swapping them, I think we should document and use the raw_ or intrinsic_ coherence explicitly, so that people know what we are talking about. Since the old method was rather confusing, I propose to deprecate it in favor of the explicit methods. But no strong opinions. What do you think?

[eleonorav89]

I think this is not a priotrity right now since, to my knowledge, the majority of cases are in high powers, high coherence, but better state it in the documentation somewhere that we are treating this type of uncertainties

[matteobachetti]

@eleonorav89 thanks for the review! I think I addressed your comments. In parallel, I'm updating the Spectral Timing notebook and others using the new functionality, I will merge after that part is ready too.

[matteobachetti]

StingraySoftware/notebooks#127