Replace direct matrix inverse with Tikhonov-regularized solve for MVAR Fourier #72
Conversation
…R Fourier - Replace xp.linalg.inv(H) with xp.linalg.solve(H + λI, I) in _MVAR_Fourier_coefficients - Replace xp.linalg.inv(H_0) with regularized solve in _estimate_transfer_function - Use scale-aware regularization: λ = 1e-12 * mean(||H||²) for stability - Add stress test for near-singular frequency bins to verify no LinAlgError This prevents numerical instability from singular or near-singular transfer function matrices that can occur with highly correlated signals. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR replaces direct matrix inversion operations in MVAR Fourier coefficient computations with Tikhonov-regularized linear solves to prevent numerical instability when dealing with near-singular frequency bins.
- Replaces
inv(H)withsolve(H + λI, I)using scale-aware regularization - Adds comprehensive stress test for near-singular cross-spectral matrices
- Applies regularization to both transfer function estimation and MVAR coefficient computation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spectral_connectivity/connectivity.py | Implements Tikhonov regularization in _MVAR_Fourier_coefficients property and _estimate_transfer_function function |
| tests/test_connectivity.py | Adds stress test test_mvar_regularized_inverse_near_singular for near-singular frequency bins |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| H = self._transfer_function | ||
| # Tikhonov regularization: solve(H + λI, I) instead of inv(H) | ||
| lam = 1e-12 * xp.mean(xp.real(xp.conj(H) * H)) # Scale-aware regularization |
There was a problem hiding this comment.
The regularization parameter lam uses a magic number 1e-12. Consider defining this as a named constant (e.g., TIKHONOV_REGULARIZATION_FACTOR = 1e-12) to improve maintainability and make it easier to adjust if needed.
Copilot uses AI. Check for mistakes.
| ) | ||
| H_0 = inverse_fourier_coefficients[..., 0:1, :, :] | ||
| # Tikhonov regularization: solve(H_0 + λI, I) instead of inv(H_0) | ||
| lam = 1e-12 * xp.mean(H_0 * H_0) # Scale-aware regularization for real matrix |
There was a problem hiding this comment.
The regularization parameter uses the same magic number 1e-12 as in the other function. Consider extracting this to a shared constant to ensure consistency and easier maintenance.
Copilot uses AI. Check for mistakes.
tests/test_connectivity.py
Outdated
| base_signal = np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) + \ | ||
| 1j * np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) |
There was a problem hiding this comment.
[nitpick] The complex signal generation spans multiple lines with backslash continuation. Consider using parentheses for line continuation instead: base_signal = (np.random.randn(...) + 1j * np.random.randn(...))
| base_signal = np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) + \ | |
| 1j * np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) | |
| base_signal = ( | |
| np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) | |
| + 1j * np.random.randn(n_time_samples, n_trials, n_tapers, n_fft_samples) | |
| ) |
Copilot uses AI. Check for mistakes.
- Break long lines in regularized inverse implementation - Fix line length issues in new stress test - Apply black formatting to ensure consistent style 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Test plan
test_mvar_regularized_inverse_near_singularpasses🤖 Generated with Claude Code