feat (sparse): add binary operators overloading #1082
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds binary operator overloading (+, -, *, /) for sparse matrix types (COO, CSR, CSC, ELL, SELLC) in the stdlib sparse module. The operators support element-wise operations between matrices of the same type, as well as mixed sparse-scalar operations.
Key changes:
- Implementation of pure functions for all four binary operators across all sparse matrix types and kinds (real/complex)
- Comprehensive unit tests for COO, CSR, and CSC matrix types (ELL and SELLC excluded due to explicit zeros causing issues with division)
- Bug fixes for CSC conversion: added
from_ijvsupport for CSC type and corrected COO sorting call for rectangular matrices incoo2csc
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stdlib_sparse_operators.fypp | New submodule implementing element-wise operator overloading for sparse matrices with three function variants per operator (matrix-matrix, scalar-matrix, matrix-scalar) |
| src/stdlib_sparse_kinds.fypp | Added operator interface declarations and public exports for the four arithmetic operators |
| src/stdlib_sparse_conversion.fypp | Added CSC from_ijv implementation and fixed dimension ordering bug in coo2csc for rectangular matrices |
| test/linalg/test_linalg_sparse.fypp | Added comprehensive test suite for operator overloading covering all operator variants for COO, CSR, and CSC types |
| doc/specs/stdlib_sparse.md | Added operator documentation section and fixed typo in interface reference (csr2sellc → csr2ell) |
| src/CMakeLists.txt | Added new operators submodule to build configuration |
| include/common.fypp | Added SELLC to the SPARSE_KINDS list for template generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1082 +/- ##
=======================================
Coverage 68.64% 68.64%
=======================================
Files 394 394
Lines 12730 12730
Branches 1376 1376
=======================================
Hits 8738 8738
Misses 3992 3992 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jvdp1
left a comment
jvdp1
left a comment
There was a problem hiding this comment.
Thank you. Useful additions. I left a few comments.
|
|
||
| #:for k1, t1, s1 in (KINDS_TYPES) | ||
| subroutine csc_from_ijv_${s1}$(CSC,row,col,data,nrows,ncols) | ||
| type(CSC_${s1}$_type), intent(inout) :: CSC |
There was a problem hiding this comment.
should it be:
| type(CSC_${s1}$_type), intent(inout) :: CSC | |
| type(CSC_${s1}$_type), intent(out) :: CSC |
There was a problem hiding this comment.
all <>_from_ijv_<> procedures define intent(inout). If we change here, we might want to change them all ?
There was a problem hiding this comment.
If in practice, it is always intent(out), I think that it would be cleaner to change them all. However, I did not check the whole code. It could be also done in another PR.
There was a problem hiding this comment.
Let's save it for a PR maybe together with #1037
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
Co-authored-by: Jeremie Vandenplas <[email protected]>
2 participants
This PR proposes support for binary operators
+, -, *, /for all sparse matrices defined in thestdlib_sparse_kindsmodule.purefunctionsELLandSELLCskipped because of explicit zeros causing NaNs at division)Additional fixes needed for this PR:
from_ijvfor theCSCtypecoo2cscin case of a rectangular matrix.