Make element total #393
Make element total #393
Conversation
|
After discovering that solvers enforce the element bounds at toplevel, we should do the same. This also simplifies handling reified element constraints |
|
We decided to enforce strict element bounds at construction time. This eliminates the need for checking the bounds in the decomposition, and any trouble with incomplete functions. |
|
Another option would be to allow the creation of such an element constraint, but enforcing the bounds at toplevel and throwing an error when trying to get the value of an element constraint with index out of bounds. |
|
Enforced that element is Total at construction time, and because of that also enforce strict bounds for Inverse and Circuit global constraints. Added tests for this and altered the existing tests to use accurate bounds. |
…-handled-correctly # Conflicts: # tests/test_globalconstraints.py
|
@Dimosts can you maybe check the changes in the chess example? As you can see this is mostly changes to our tests and examples, so that they use strict bounds |
| lb = min(lbs) | ||
| ub = max(ubs) | ||
| if lb < 0 or ub >= len(flatargs): | ||
| raise TypeError("Circuit global constraint is only defined if all bounds are between 0 and the length of the array.") |
There was a problem hiding this comment.
I wouldn't call it a type error, but rather a PartialFunctionError or something? I think we even have one already
There was a problem hiding this comment.
Maybe make the error message more explicit too?
Smt like: elements in the array must have bounds 0 <= x <= len(flatargs) but got bounds [{lb},{ub}] for arg ...
| lb = min(lbs) | ||
| ub = max(ubs) | ||
| if lb < 0 or ub >= len(fwd): | ||
| raise TypeError("All arguments must be within array bounds for Inverse to have meaning over the whole domain!") |
There was a problem hiding this comment.
Also here can be more explicit
|
I'm wondering if we should have a "vectorized check" util function or something as we are duplicating code quite a bit for these checks. It would take as input a (nested) list of elements and a boolean function which has to be checked for each of the args. |
|
Yes that could be nice, it's a bit ugly now, the reason I put it there is so the work happens once you already know there is a bounds issue (with an easier check), so that you don't have the overhead on normal inputs. |
Wout4
changed the title
No description provided.