fix: Sign error in greater_than constraint #334
Conversation
`greater_than(terms, rhs)` is implemented as `greater_than_or_equals(terms, rhs - 1)`, but should be `greater_than_or_equals(terms, rhs - 1)`. This is likely a copy-paste error from the implementation of `less_than`. Note: I've added a unit test saying that "x > 0" is not satisfiable for an integer variable x which is required to be 0. This feels like an odd test to have. I'm happy to remove or change it.
ImkoMarijnissen
left a comment
ImkoMarijnissen
left a comment
There was a problem hiding this comment.
Great catch! Thank you for creating a PR with the fix and a test case as well.
I think this brings up a good point that our MiniZinc interface is quite well-tested, but that the Rust interface requires more testing.
I have some small comments about the test case, but other than that, it looks good!
ImkoMarijnissen
changed the title
geraschenko
force-pushed
the
anton/greater_than
branch
from
4791f43 to
8d00c8b
Compare
ImkoMarijnissen
left a comment
ImkoMarijnissen
left a comment
There was a problem hiding this comment.
Great, thank you for making the changes!
It appears that the linting CI does not pass; once that is fixed, it should be good to go.
geraschenko
force-pushed
the
anton/greater_than
branch
from
8d00c8b to
d34df48
Compare
Whoops, I must have forgotten to cargo fmt. CI is now passing. "Dismiss old reviews when re-requested" is still failing. I'm not entirely sure what this is, but given that the error code is 403, I assume this is something a repo owner has to handle. |
2 participants
greater_than(terms, rhs)is implemented asgreater_than_or_equals(terms, rhs - 1), but should begreater_than_or_equals(terms, rhs + 1). This is likely a copy-paste error from the implementation ofless_than.Note: I've added a unit test saying that "x > 0" is not satisfiable for an integer variable x which is required to be 0. This feels like an odd test to have. I'm happy to remove or change it.