Repository navigation
Conversation
Wrap the create/release sequence in SumExpr, ProdExpr, and the seven unary sub-expressions in try/finally so partial failures do not leak either the parent expression or already-built children. Apply the same pattern in _createConsGenNonlinear so a failing SCIPcreateConsNonlinear no longer leaks the built scip_expr. While in the area, tighten local index/count types (Py_ssize_t for container indexing, int for SCIP nchildren) in the new code and drop the PyCons intermediate variable in _createConsGenNonlinear.
The n_ prefix matches neighboring counters (n, nvars, nbranchvars) and makes it explicit that the variable is a count rather than a boolean state. Switch kwargs[...] in _createConsGenNonlinear to double quotes for consistency with the surrounding code.
The index i was left at len(children)-1 after the child loop, so the constant component overwrote the last child's pointer and left the final slot (which SCIPcreateExprSum reads because n=len+1) with uninitialized memory. Initialize i to len(children) before writing the constant. Fixes the segfaults in test_nonlinear, test_recipe_nonlinear, and test_matrix_variable when a GenExpr with nonzero constant is added to a constraint or objective via _createConsGenNonlinear.
…asses SumExpr, ProdExpr, PowExpr, and Constant had the ordering reversed from the six UnaryExpr subclasses (Abs/Exp/Log/Sqrt/Sin/Cos) and from VarExpr, which already put to_scip_expr first. Align them all so the new to_scip_expr method appears above the pre-existing _evaluate in every GenExpr class.
Abs/Exp/Log/Sqrt/Sin/Cos had __repr__ (and Abs's __abs__) sitting between to_scip_expr and _evaluate. Move the dunder methods above to_scip_expr so all 11 GenExpr subclasses follow the same layout: init/dunders, then to_scip_expr, then _evaluate, then copy.
This branch has not been deployed
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.
Replace the two-pass general expression conversion (
expr_to_nodes/expr_to_array/value_to_arraybuilding an intermediatelist[(type, indices)]node table and then walking it) with per-classto_scip_expr(self, scip)recursion onGenExprand its subclasses.Also hardens the exception paths so an error raised inside
SCIPcreateExpr*no longer leaks already-built childSCIP_EXPRreferences.