Skip to content

Fix constants in Xor#778

Closed
IgnaceBleukx wants to merge 12 commits intomasterfrom
xor_simplify_bool
Closed

Fix constants in Xor#778
IgnaceBleukx wants to merge 12 commits intomasterfrom
xor_simplify_bool

Conversation

@IgnaceBleukx
Copy link
Copy Markdown
Collaborator

Some changes to how we handle constants in Xor, fixing both #620 and #747

Trivially true comparisons are now simplified away during simplify_bool, and we cann simplify_bool on the decomposition of Xor.
This is not needed for any other global constraint, as it is the only one that takes Boolean arguments.
(Actually also IfThenElse does, but it's decomposition uses implies constraints, and the simplifications are built-in to that constructor)

@IgnaceBleukx IgnaceBleukx requested a review from ThomSerg October 24, 2025 07:14
@ThomSerg
Copy link
Copy Markdown
Collaborator

Looks good to me, waiting for the tests to finish.

@IgnaceBleukx
Copy link
Copy Markdown
Collaborator Author

I was a bit too quick with adding the trivially true/false cases in simplify_bool.
Implemented it properly now by computing bounds on the comparison itself.

Can probably do with a re-review

Copy link
Copy Markdown
Collaborator

@ThomSerg ThomSerg left a comment

Choose a reason for hiding this comment

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

Maybe a small change to the test suite.

Also some tests seem to be failing due to the simplification now resulting in different expressions after transformation. The hard-coded checks fail.

Comment thread tests/test_expressions.py
elif lb == 1 == ub:
self.assertEqual(cp.Model(expr).solveAll(), total_vals)
else:
self.assertNotEqual(cp.Model(expr).solveAll(), total_vals)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use assertLess()?
What with assertMore(..., 0)?

@ThomSerg ThomSerg mentioned this pull request Jan 16, 2026
@tias tias mentioned this pull request Apr 4, 2026
@tias
Copy link
Copy Markdown
Collaborator

tias commented Apr 21, 2026

solved in #908

what remains in this PR is that the bounds of Comparison now check for trivial true/falseness; we do not currently plan to put this optimisation in main

@tias tias closed this Apr 21, 2026
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.

3 participants