Skip to content

fix(signal_detrend): correct D_2 matrix in tarvainen2002 method - #1198

Open
LensHRV wants to merge 3 commits into
neuropsychology:masterfrom
LensHRV:fix-tarvainen2002-d2-matrix
Open

fix(signal_detrend): correct D_2 matrix in tarvainen2002 method#1198
LensHRV wants to merge 3 commits into
neuropsychology:masterfrom
LensHRV:fix-tarvainen2002-d2-matrix

Conversation

@LensHRV

@LensHRV LensHRV commented Aug 3, 2026

Copy link
Copy Markdown

This PR fixes an issue in _signal_detrend_tarvainen2002 where the second-difference matrix $D_2$ was being truncated at the boundaries due to the usage of scipy.sparse.dia_matrix with rectangular shapes $(N-2) \times N$. This caused the last entries of the detrended residual to artificially diverge or evaluate to zero.

Summary of Improvements:

  • Bug Fix ($D_2$ Matrix): Replaced scipy.sparse.dia_matrix with scipy.sparse.diags([1, -2, 1], [0, 1, 2], shape=(N - 2, N)). This ensures the $D_2$ operator retains its complete [1, -2, 1] coefficients across all rows without truncating edge elements.
  • Performance & Numerical Stability: Replaced np.linalg.inv with np.linalg.solve(A, signal). Following the official NumPy and SciPy documentation recommendations, using a direct linear solver avoids the computational overhead and numerical instability of explicit matrix inversion. It also eliminates unnecessary intermediate 2D array allocations (B, .T), improving execution speed and memory footprint.

Validation:

  • The output residual now matches the analytical solution from Tarvainen et al. (2002) to machine precision (atol < 1e-9).
  • The boundary artifact issue (where the last two residual points evaluated to 0) is fully resolved.

References:

  • Tarvainen, M. P., Ranta-Aho, P. O., & Karjalainen, P. A. (2002). An advanced detrending method with application to HRV analysis. IEEE Transactions on Biomedical Engineering, 49(2), 172-175.
  • Official NumPy linalg.inv Documentation: "Calculating the inverse of a matrix is computationally expensive and numerically unstable. To solve a linear system $Ax = b$, use numpy.linalg.solve(A, b) instead."

Fixes boundary truncation in D_2 matrix using scipy.sparse.diags and improves numerical stability and performance using np.linalg.solve.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.77%. Comparing base (ff419d9) to head (be50e1c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1198      +/-   ##
==========================================
- Coverage   57.78%   57.77%   -0.01%     
==========================================
  Files         310      310              
  Lines       15680    15678       -2     
==========================================
- Hits         9060     9058       -2     
  Misses       6620     6620              

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants