Wrap captureCons and releaseCons#1221
Conversation
a3084ce to
d0d1381
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds Python-level wrappers for SCIP’s constraint reference counting so advanced users can explicitly capture/release constraints, and updates typing/tests/changelog accordingly.
Changes:
- Added
Model.captureCons()andModel.releaseCons()wrappers in the Cython layer. - Added a unit test covering basic usage and TypeError behavior.
- Updated the type stub (
scip.pyi) andCHANGELOG.mdto reflect the new API.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_cons.py | Adds a new test for captureCons()/releaseCons() and invalid input handling. |
| src/pyscipopt/scip.pyi | Exposes the new methods in the public type stub. |
| src/pyscipopt/scip.pxi | Implements the new wrappers around SCIPcaptureCons / SCIPreleaseCons. |
| CHANGELOG.md | Documents the newly added API methods under “Unreleased”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Could this also be done for variables to avoid asymmetry? |
|
@DominikKamp done |
|
|
||
| Returns | ||
| ------- | ||
| int |
There was a problem hiding this comment.
This seems incomplete.
|
|
||
| Returns | ||
| ------- | ||
| int |
There was a problem hiding this comment.
This seems incomplete.
DominikKamp
left a comment
There was a problem hiding this comment.
I do not understand, what the changes in scip.pyi have to do with captures, otherwise this looks okay.
|
What is the use case that this enables? are you sure you want to expose memory handling? misuse of these can expose memory unsafety and memory leaks. |
DominikKamp
left a comment
There was a problem hiding this comment.
This looks okay, but maybe wait until the usecase is reliable. So far this only introduces user unsafety. The memory should be fine since proper programs do not leave captures unreleased, same for events.
|
@mmghannam the usecase is scip#201, but apparently it's still being discussed. @mmghannam @DominikKamp , in the in-person meeting Marc (Pfetsch) wished for an Perhaps such a separation would make sense also for more advanced use cases. For example, we've add users thinking they had to use Does this make any sense or is it time for me to go to bed? |
|
Maybe there could be a base class with elementary functionality, but for these functions a warning label should be sufficient, so better go to bed at first. |
No description provided.