BUG: can't call getVal from GenExpr
#1148
Open
Conversation
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
Introduces the test_evaluate function to verify correct evaluation of various expressions using Model.getVal, including arithmetic and trigonometric operations, and checks for TypeError on invalid input.
Introduced _evaluate methods to Term, Expr, and all GenExpr subclasses for consistent evaluation of expressions and variables. Refactored Solution and Model to use these methods, simplifying and unifying value retrieval for variables and expressions. Cleaned up class definitions and improved hashing and equality for Term and Variable.
Replaced explicit loop with flat iterator for evaluating matrix expressions in Solution.getVal, improving performance and code clarity.
Introduces a new test 'test_evaluate' to verify correct evaluation of matrix variable division and summation in the model.
Eliminated special handling for MatrixExpr and MatrixGenExpr in Solution.__getitem__, simplifying the method to only evaluate single expressions. This change streamlines the code and removes unnecessary numpy array construction.
Renamed 'vars' to 'vartuple' and '_hash' to 'hashval' in the Term class for clarity. Updated all references and methods to use the new attribute names, improving code readability and consistency.
Changed internal evaluation methods in expression classes from float to double for improved numerical precision. Updated type declarations and imports accordingly.
Added type annotations to the Term constructor and specified types for class attributes. This improves code clarity and type safety.
Changed all _evaluate methods in expression classes from cdef to cpdef to make them accessible from Python. Moved the definition of terms and _evaluate to the Expr class in scip.pxd, and refactored Variable to inherit from Expr in scip.pxd, consolidating class member definitions. These changes improve the interface and maintainability of expression evaluation.
Zeroto521
commented
Jan 8, 2026
| import math | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| from pyscipopt.scip cimport Variable, Solution |
Contributor
Author
There was a problem hiding this comment.
This could let IDE know what Variable and Solution are.
Zeroto521
commented
Jan 8, 2026
| include "matrix.pxi" | ||
|
|
||
| if TYPE_CHECKING: | ||
| double = float |
Zeroto521
commented
Jan 8, 2026
|
|
||
|
|
||
| class Term: | ||
| cdef class Term: |
Contributor
Author
There was a problem hiding this comment.
The cdef method should stay in the cdef class.
Ensures that MatrixExpr._evaluate returns a numpy ndarray when appropriate. Adds a test to verify the return type of getVal for matrix variables.
Deleted the _evaluate method from the Solution class in the scip.pyi stub file, as it is no longer needed. Also added TYPE_CHECKING to the typing imports.
Applied the @disjoint_base decorator to the Term and UnaryExpr classes in scip.pyi to clarify their base class relationships. This may improve type checking or class hierarchy handling.
Appended '# noqa: F401' to the 'ClassVar' import to suppress linter warnings about unused imports in scip.pyi.
Zeroto521
commented
Jan 8, 2026
| def __matmul__(self, other): | ||
| return super().__matmul__(other).view(MatrixExpr) | ||
|
|
||
| def _evaluate(self, Solution sol) -> NDArray[np.float64]: |
Contributor
Author
There was a problem hiding this comment.
np.ndarray is not a cdef class, so it can't convert MatrixExpr to a cdef class.
And in MatrixExpr, _evaluate is a pure Python function.
Replaces the @np.vectorize-decorated _evaluate function with an np.frompyfunc-based implementation for evaluating expressions with solutions. This change may improve compatibility and performance when applying _evaluate to arrays.
Refactored the _evaluate method in MatrixExpr to always return the result as a NumPy ndarray, removing the conditional type check.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1148 +/- ##
==========================================
+ Coverage 55.07% 55.12% +0.04%
==========================================
Files 24 24
Lines 5420 5448 +28
==========================================
+ Hits 2985 3003 +18
- Misses 2435 2445 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
to fix #1062