-
Notifications
You must be signed in to change notification settings - Fork 274
getTermsQuadractic correctly returns linear terms #1132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #1083 where getTermsQuadratic() was not correctly returning all linear terms for variables that also appear in quadratic or bilinear terms. The fix stores the original polynomial expression in the constraint's data attribute and uses it to reconstruct all linear coefficients. Additionally, the PR adds a new getVarFarkasCoef() method for retrieving Farkas coefficients, though this remains untested.
Key Changes
- Modified
getTermsQuadratic()to return all linear coefficients by recovering them from the original expression stored inConstraint.data - Added storage of original
Exprobjects in quadratic constraints to preserve full coefficient information - Added new
getVarFarkasCoef()method with declaration in the header file
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| tests/test_nonlinear.py | Adds comprehensive test for mixed linear/quadratic constraints to validate that getTermsQuadratic() now correctly returns all linear terms |
| src/pyscipopt/scip.pxi | Implements the fix by storing original expressions in constraint data and extracting linear terms from them; adds new getVarFarkasCoef() method |
| src/pyscipopt/scip.pxd | Declares the external SCIP function SCIPgetVarFarkasCoef |
| CHANGELOG.md | Documents the fix and addition of the new method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # even if SCIP's internal quadratic representation does not expose | ||
| # all linear coefficients explicitly. | ||
| if PyCons.data is None: | ||
| PyCons.data = quadcons.expr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot get the data from SCIP instead of this added complexity?
Fix #1083
I also added
getVarFarkasCoef()because I was to lazy to rebase. Still no test, as a more convoluted pricer would need to be implemented (see the end of the previous sentence).