fix: Added full_matrices parameter to dask.array.linalg.svd#12292
fix: Added full_matrices parameter to dask.array.linalg.svd#12292jacobtomlinson merged 5 commits intodask:mainfrom
dask.array.linalg.svd#12292Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the full_matrices parameter to dask.array.linalg.svd to improve conformance with the Array API specification, addressing issue #10389. The implementation exposes full_matrices=False (reduced SVD, which Dask already computes) and raises NotImplementedError for full_matrices=True (full SVD, which is not feasible with chunked arrays). This allows Array API-consuming libraries like scikit-learn to use Dask arrays while staying backend-agnostic.
Changes:
- Added
full_matricesparameter tosvd()function signature with default valueFalseto maintain backward compatibility - Added validation logic that raises
NotImplementedErrorwhenfull_matrices=True - Added comprehensive tests covering both the error case and the working case
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| dask/array/linalg.py | Added full_matrices parameter to svd() function with validation logic and updated docstring |
| dask/array/tests/test_linalg.py | Added tests for full_matrices=True raising NotImplementedError and full_matrices=False working correctly |
| .gitignore | Added dev directory to ignore list (appears unrelated to PR purpose) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 21 files ± 0 21 suites ±0 5h 3m 6s ⏱️ +8s For more details on these failures, see this check. Results for commit a8b7544. ± Comparison against base commit 4195f5a. ♻️ This comment has been updated with latest results. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
CI failure looks unrelated |
dask.array.linalg.svddoes not conform to the Array API spec #10389pre-commit run --all-filesAdd full_matrices keyword argument to
dask.array.linalg.svdfor Array API spec conformance. Dask's SVD already computes the reduced SVD (full_matrices=False), but the parameter was not exposed, causingTypeErrorwhen Array API-consuming libraries (e.g., scikit-learn) passed it explicitly.full_matrices=Falseworks as before.full_matrices=TrueraisesNotImplementedErrorsince full SVD is not feasible with chunked arrays. Default isFalseto maintain backward compatibility.