Skip to content

Interface SCIP logicor constraints - #1255

Merged
Joao-Dionisio merged 2 commits into
scipopt:masterfrom
mohitt31:logicor-constraints
Sep 15, 2026
Merged

Joao-Dionisio merged 2 commits into
scipopt:masterfrom
mohitt31:logicor-constraints

Conversation

@mohitt31

Copy link
Copy Markdown
Contributor

Adds the logicor constraint handler to the Python interface. It is one of the unchecked items in #977.

Methods added:

  • addConsLogicor(vars, ...) creates the constraint
  • addCoefLogicor(cons, var) adds a variable to an existing one
  • getNVarsLogicor(cons) and getVarsLogicor(cons)
  • getDualsolLogicor(cons) and getDualfarkasLogicor(cons)

They follow the same shape as the existing AND and OR handlers, so the keyword arguments and their defaults match addConsAnd.

Tested with SCIP 10.0.3 on macOS (arm64). The suite goes from 449 to 450 passed with the same skips, and stubs/test.sh reports no issues against the stubs I added.

One thing I ran into while writing the handler check in getVarsLogicor. The equivalent line in getVarsAnd (scip.pxi:6956) is

assert(constype == 'and', "The constraint handler %s does not have this functionality." % constype)

That asserts a two element tuple, which is always truthy, so the guard never actually fires. I wrote mine without the parentheses. I left the AND one alone to keep this PR to one thing, but happy to send a separate fix if you want it.

Adds addConsLogicor, addCoefLogicor, getNVarsLogicor, getVarsLogicor,
getDualsolLogicor and getDualfarkasLogicor, following the same shape as the
existing AND and OR handlers. Part of the checklist in scipopt#977.
@Joao-Dionisio

Copy link
Copy Markdown
Member

Hello @mohitt31 , thank you so much! I'll only be able to take a look at this over the next week, I believe, but it's looking good!

Comment thread src/pyscipopt/scip.pxi Outdated
@Joao-Dionisio

Copy link
Copy Markdown
Member

happy to send a separate fix if you want it.

I would also be happy if you did it, if you don't mind 😄
We have another issue in getDualSolKnapsack():

        if cons.isOriginal():
            transcons = cons
        else:
            transcons = <Constraint>self.getTransformedCons(cons)

This is the wrong way round, of course, and is also present in the other farkas function. Would you mind also fixing this in that separate fix?

SCIPgetDualsolLogicor and SCIPgetDualfarkasLogicor need the transformed
constraint, so follow getDualsolLinear and resolve it when an original one
is passed in. Both now also reject a constraint of the wrong type rather
than returning a meaningless number.
@mohitt31

Copy link
Copy Markdown
Contributor Author

Sure, I'll do it. I checked and getDualfarkasKnapsack has the same problem, it passes the original constraint straight to SCIP. So the separate PR will fix both knapsack functions and the getVarsAnd assert, with tests that call them on original constraints. I'll open it in the next day or two.

@Joao-Dionisio Joao-Dionisio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mohitt31 ! I'll take a look at your other PR later, maybe this week.

@Joao-Dionisio
Joao-Dionisio merged commit 516a7cd into scipopt:master Sep 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants