Speed up MatrixExpr.add.reduce via quicksum#1157
Merged
Joao-Dionisio merged 21 commits intoscipopt:masterfrom Jan 20, 2026
Merged
Speed up MatrixExpr.add.reduce via quicksum#1157Joao-Dionisio merged 21 commits intoscipopt:masterfrom
MatrixExpr.add.reduce via quicksum#1157Joao-Dionisio merged 21 commits intoscipopt:masterfrom
Conversation
Implements the __array_ufunc__ method in MatrixExpr to handle numpy ufuncs, specifically enabling correct behavior for reductions like np.add.reduce by delegating to the sum method. This improves compatibility with numpy operations.
Moved the sum computation logic from MatrixExpr.sum and __array_ufunc__ into a new _core_sum function for better code reuse and maintainability.
Introduces tests to compare the performance of the mean operation on matrix variables and checks the return types of mean with and without axis argument.
Documented the speed improvement for MatrixExpr.add.reduce using quicksum in the changelog.
The sum method was removed from the MatrixExpr class, consolidating summation logic in the _core_sum function. The docstring for _core_sum was expanded to include detailed parameter and return value descriptions, improving code clarity and maintainability.
Zeroto521
commented
Jan 16, 2026
Deleted the type stub for the sum method in MatrixExpr, likely to reflect changes in the underlying implementation or to correct type information.
Enhanced the __array_ufunc__ method in MatrixExpr to ensure proper array conversion and consistent return types. Added the _ensure_array helper for argument handling. Also removed an unused include of matrix.pxi from expr.pxi.
Zeroto521
commented
Jan 16, 2026
Updated MatrixExpr.__matmul__ to return the correct type when the result is not an ndarray. Adjusted tests to reflect the expected return type for 1D matrix multiplication and improved performance test timing logic.
Simplifies timing measurement in matrix sum performance tests by directly calculating elapsed time instead of storing start and end times separately. This improves code readability and reduces variable usage.
Changed the expected type of 1D @ 1D matrix multiplication from MatrixExpr to Expr in test_matrix_matmul_return_type to reflect updated behavior.
Updated tests to use x.view(MatrixExpr) and x.view(np.ndarray) instead of direct subclass method calls. This clarifies the intent and ensures the correct method resolution for sum and mean operations in performance and result comparison tests.
Zeroto521
commented
Jan 16, 2026
Zeroto521
commented
Jan 16, 2026
Member
|
Oh @Zeroto521 , I've been forgetting about the tutorials! Can you please take a look at |
Reorganize logic in MatrixExpr.__array_ufunc__ to handle the 'reduce' method and argument conversion more clearly. This improves readability and ensures correct handling of the 'out' keyword and argument conversion only when necessary.
Contributor
Author
I see that. The current matrix only has performance changes. We can add some introduction about this later. |
Enhanced the MatrixExpr.__array_ufunc__ method with detailed type annotations and a comprehensive docstring. This improves code clarity and developer experience when working with custom NumPy ufunc behavior.
A comment was added to explain that the 'reduce' method handles reductions like np.sum(a), improving code readability.
Member
|
@Zeroto521 can you please fix the conflicts that arose from merging the other PR? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes MatrixExpr array reduction operations by implementing __array_ufunc__ to intercept np.add.reduce calls and route them through the optimized quicksum function. This approach automatically speeds up sum(), mean(), and other operations that internally use add.reduce.
Changes:
- Replaced the custom
summethod with__array_ufunc__to interceptnp.add.reduceoperations - Refactored the sum logic into a standalone
_core_sumfunction - Updated
__matmul__to correctly handle scalar vs array results - Added tests for mean functionality and updated existing tests to use proper API patterns
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pyscipopt/matrix.pxi | Implements __array_ufunc__ to intercept np.add.reduce and route to optimized quicksum; refactors sum logic to _core_sum; adds _ensure_array helper; improves __matmul__ return type handling |
| tests/test_matrix_variable.py | Updates tests to use public API (e[CONST] instead of e.terms[CONST]); modifies performance tests to use view(np.ndarray); adds mean functionality tests |
| src/pyscipopt/scip.pyi | Removes explicit sum method type hint since now handled via __array_ufunc__ |
| src/pyscipopt/expr.pxi | Removes redundant include "matrix.pxi" (still included in scip.pxi) |
| CHANGELOG.md | Documents the performance optimization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Updated test_matrix_sum_result to extract and cast the result of the MatrixExpr sum operation before assertions. This improves clarity and ensures type consistency between the expected and actual results.
Updates the __array_ufunc__ method in MatrixExpr to ensure all elements in the 'out' tuple are unboxed with _ensure_array, preventing recursion issues when 'out' is provided as a tuple.
Updated comments in test_matrix_mean_performance to correctly refer to 'mean' instead of 'sum', reflecting the actual operations being performed in the test.
Contributor
Author
Done |
Joao-Dionisio
approved these changes
Jan 20, 2026
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.
closes to #1153
np.add.reduceis the inner function fornp.ndarray.sum,np.ndarray.mean, and more.