Skip to content

Fix numba functions in intrinsic coherence#970

Open
matteobachetti wants to merge 6 commits intomainfrom
fix_numba_funcs
Open

Fix numba functions in intrinsic coherence#970
matteobachetti wants to merge 6 commits intomainfrom
fix_numba_funcs

Conversation

@matteobachetti
Copy link
Member

#965 introduced a bug that made Numba never loaded for Stingray. It depended on a misinterpretation of how UniTuples work in vectorized functions.

Sadly, I needed to rewrite some of the private functions in order not to incur in Numba's wrath for unclear data types. This meant, for example, having all data passed as arrays if they could be either arrays or scalars (i.e. transforming scalars in 1-d arrays, e.g. in _intrinsic_coherence_array). Also, it meant that all warnings had to be thrown outside numba, which forced me, e.g., to introduce the flagged list for intrinsic_coherence with adjust_bias=True

@codecov
Copy link

codecov bot commented Mar 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.23%. Comparing base (75c43a9) to head (14aef0e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #970      +/-   ##
==========================================
+ Coverage   95.49%   96.23%   +0.73%     
==========================================
  Files          48       48              
  Lines       10062    10098      +36     
==========================================
+ Hits         9609     9718     +109     
+ Misses        453      380      -73     

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug introduced in PR #965 where Numba was never loaded for Stingray due to a misuse of UniTuple in vectorized functions. The fix rewrites several private coherence functions to use string-based Numba type signatures, removes the UniTuple dependency, introduces a new array-level helper function for intrinsic coherence calculation, and moves Python-level concerns (such as warnings) outside of Numba-compiled functions.

Changes:

  • stingray/utils.py: Splits Numba imports into a two-step pattern to fix the loading bug; removes UniTuple from the fallback stubs.
  • stingray/fourier.py: Rewrites _bias_term, _raw_coherence, _intrinsic_coherence, _intrinsic_coherence_with_adjusted_bias, and check_powers_for_intrinsic_coherence to use string-based Numba type signatures; adds new intrinsic_coherence_array as a @njit helper; moves convergence warnings out of Numba-compiled code.
  • stingray/tests/test_fourier.py: Adds tests for array-shaped noise/n_ave inputs and updates the check_powers_for_intrinsic_coherence call to pass the now-mandatory threshold argument.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
stingray/utils.py Splits Numba imports into a two-phase pattern (try/except for detection, if HAS_NUMBA: block for full imports); removes UniTuple stub
stingray/fourier.py Replaces UniTuple-based vectorize signatures with string-based ones; refactors _intrinsic_coherence_with_adjusted_bias to return a convergence flag; adds new intrinsic_coherence_array helper; adds debug print statement (unintentional)
stingray/tests/test_fourier.py Adds test for array-valued noise and n_ave inputs; updates test to pass mandatory threshold arg
docs/changes/970.bugfix.rst Adds changelog entry for the bug fix

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.


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

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.

2 participants