Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new test module validating cvx.linalg.ewm_covariance against both pandas’ EWM covariance and Basanos’ existing ewm_corr, and simultaneously migrates Basanos’ linear-algebra helpers/exceptions from internal implementations to the external cvx-linalg package.
Changes:
- Add
tests/test_math/test_ewm_covariance.pyto comparecvx.linalg.ewm_covariancevs pandas EWM covariance and (dense-only) vs Basanosewm_corr. - Replace internal
_linalgusage withcvx.linalg(solve,inv_a_norm,valid, and matrix-related exceptions), and deletesrc/basanos/math/_linalg.pyplus its dedicated test modules. - Update packaging to depend on
cvx-linalg>=0.5and adjust dependency constraints.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Adds cvx-linalg and updates locked metadata/requirements. |
pyproject.toml |
Adds cvx-linalg runtime dependency and updates other dependency minimums; adds deptry module map. |
src/basanos/exceptions.py |
Removes linear-algebra exceptions from Basanos’ exception module and updates docstring guidance. |
src/basanos/__init__.py |
Re-exports selected cvx.linalg exceptions from the top-level basanos namespace. |
src/basanos/math/_engine_solve.py |
Switches solver/norm/error imports from internal _linalg to cvx.linalg. |
src/basanos/math/_engine_diagnostics.py |
Switches valid/solve and SingularMatrixError imports to cvx.linalg. |
src/basanos/math/_factor_model.py |
Switches linalg utilities/exceptions to cvx.linalg. |
src/basanos/math/_stream.py |
Switches SingularMatrixError import to cvx.linalg. |
src/basanos/math/_linalg.py |
Deletes internal linear-algebra helper implementation. |
tests/test_math/test_ewm_covariance.py |
New test suite for ewm_covariance vs pandas and vs Basanos ewm_corr (dense-only). |
tests/test_math/test_numerical_stability.py |
Updates imports to use cvx.linalg for solver/warning/threshold. |
tests/test_math/test_factor_model.py |
Updates imports to use cvx.linalg for exception/warning/threshold. |
tests/test_math/test_optimizer.py |
Updates SingularMatrixError import to cvx.linalg. |
tests/test_math/test_optimizer_edge_cases.py |
Updates SingularMatrixError import to cvx.linalg. |
tests/test_math/test_stream.py |
Updates SingularMatrixError imports to cvx.linalg. |
tests/test_math/test_linalg.py |
Deletes unit tests for the removed internal _linalg module. |
tests/test_math/test_linalg_property.py |
Deletes Hypothesis property tests for the removed internal _linalg module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
8
to
12
|
|
||
| Examples: | ||
| >>> raise NonSquareMatrixError(3, 2) | ||
| Traceback (most recent call last): | ||
| ... | ||
| basanos.exceptions.NonSquareMatrixError: Matrix must be square, got shape (3, 2). | ||
| Linear-algebra exceptions (``NonSquareMatrixError``, ``DimensionMismatchError``, | ||
| ``SingularMatrixError``, ``IllConditionedMatrixWarning``) are defined in | ||
| ``cvx.linalg`` and should be imported from there directly. | ||
| """ |
Comment on lines
11
to
16
| dependencies = [ | ||
| "jinja2>=3.1", | ||
| "jquantstats>=0.2.0", | ||
| "cvx-linalg>=0.5", | ||
| "jinja2>=3", | ||
| "jquantstats>=0.9", | ||
| "numpy>=2.4", | ||
| "plotly>=6.6.0", |
Comment on lines
27
to
+31
| import numpy as np | ||
| import polars as pl | ||
| import pytest | ||
| from cvx.linalg import IllConditionedMatrixWarning, solve | ||
| from cvx.linalg.solve import _DEFAULT_COND_THRESHOLD |
Comment on lines
+4
to
+5
| 1. ewm_covariance matches pandas.DataFrame.ewm(span).cov(bias=True) to 1e-10 | ||
| wherever both produce finite values. |
Comment on lines
+19
to
23
| from cvx.linalg import DimensionMismatchError, SingularMatrixError | ||
| from cvx.linalg import check_and_warn_condition as _check_and_warn_condition | ||
| from cvx.linalg import cholesky_solve as _cholesky_solve | ||
| from cvx.linalg.solve import _DEFAULT_COND_THRESHOLD | ||
|
|
tschm
changed the title
Add cvx-linalg>=0.4 as a dependency and remove the now-redundant basanos/math/_linalg.py. All linalg types (SingularMatrixError, DimensionMismatchError, NonSquareMatrixError, IllConditionedMatrixWarning, cholesky_solve, is_positive_definite, solve, valid, inv_a_norm) are imported from cvx.linalg throughout src and tests. Update jquantstats to >=0.9.1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add test_ewm_covariance.py documenting that cvx.linalg.ewm_covariance matches pandas.ewm(span).cov(bias=True) to machine epsilon on dense and sparse data. Also verifies that normalised covariance matches ewm_corr exactly on dense inputs, and documents the two known semantic differences for sparse data: conservative NaN pattern and marginal-vs-joint variance in the normalisation denominator. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_covariance Deletes the hand-rolled ewm_corr implementation and replaces it everywhere with the cvx-linalg ewm_covariance function, which matches pandas ewm(span).cov(bias=True) to 1e-10. - Remove src/basanos/math/_ewm_corr.py and its tests/notebook - Update _engine_solve.py: WarmupState drops corr_iir_state field - Update exceptions.py doctest key name (corr_zi_x -> corr_ret_buf) - Regenerate cor_tensor.npy fixture - Remove ewm_corr-specific numerical-stability tests (covered by ewm_covariance tests) - Update test_stream.py for new _StreamState shape (format version 3) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Also adapts to cvx-linalg 0.6.x API: ewm_covariance moved to cvx.linalg.ewm_cov submodule, cholesky_solve replaced by inv + solve. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Cast corr_ret_buf to ndarray at EWM-only call sites in _stream.py to satisfy ty's union narrowing - Remove extra blank line in exceptions.py (ruff format) - Remove deleted ewm_benchmark.py from README notebooks table and its dead uv run example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/basanos/math/_ewm_corr.py(hand-rolled EWM correlation) and all associated tests/notebookoptimizer.py,_stream.py,_engine_solve.py) withcvx.linalg.ewm_covariance, which matchespandas.ewm(span).cov(bias=True)to 1e-10_SAVE_FORMAT_VERSIONto 3 (incompatible_StreamStateschema: 5 IIR fields → singlecorr_ret_bufbuffer)tests/test_math/test_ewm_covariance.pyverifying numerical agreement with pandas on dense and sparse dataTest plan
pytest tests/test_math/test_ewm_covariance.py— dense exact match, sparse superset-NaN + value match🤖 Generated with Claude Code