Fix numba functions in intrinsic coherence#970
Conversation
…changes from missing the entire library)
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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; removesUniTuplefrom the fallback stubs.stingray/fourier.py: Rewrites_bias_term,_raw_coherence,_intrinsic_coherence,_intrinsic_coherence_with_adjusted_bias, andcheck_powers_for_intrinsic_coherenceto use string-based Numba type signatures; adds newintrinsic_coherence_arrayas a@njithelper; moves convergence warnings out of Numba-compiled code.stingray/tests/test_fourier.py: Adds tests for array-shaped noise/n_ave inputs and updates thecheck_powers_for_intrinsic_coherencecall to pass the now-mandatorythresholdargument.
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.
f54c908 to
14aef0e
Compare
There was a problem hiding this comment.
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.
#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