Conversation
|
decomp custom should be an optional argument. The decomp_linear function is nice; the fact that you add a decomp_custom argument, do you prefer that over https://cpmpy.readthedocs.io/en/latest/api/expressions/globalconstraints.html#alternative-decompositions that overwrites the decomp function? I actually think the proposed approach is nice and it being explicit is a plus. Should we have separate overrides for reified? E.g. circuit toplevel is simpler... or would we achieve the equivalent when we do context aware flattening on the full formulation? |
|
I think the main advantage of using the I would have to think about the reif vs non-reif ones. As long as our decompositions also model the negation of the global, there is no real point in making the distinction; not sure if it would get that much easier if we don't model the negation in the Circuit decomp? Indeed, we should get a lot of things for free when we add context aware flattening |
|
Thought some more about it; your proposal is the better way forward. And we don't need answers to the half-reif question to move forward with this improvement to our current linearisation. |
|
Merged the current master in this branch and added some tests. One open question and one issue remains:
|
|
Ok managed to fix the element issue, small oversight in the decomposition on my side ; ) What other globals would we want to make a linear friendly decomposition for? Also the the ExceptN variants probably? |
|
Finally got the tests to work and added some extra globals. Should be ready for review now. |
|
Great to see this happening. Why do you use encode/intvarmap instead of the more straightforward (var == val) expressions, which CSE has to pick up later then? (this would push the responsability of handling the proper encoding of the var and the 'keep_integer' to later in the stack) |
|
chatted with an agent about my rough ideas; decided a new transformation instead of doing two-stage could work well. Small test example: gives We could give the new transformation your original |
|
I think the keep_integer is dangerous... it relies on removing the integer later. Perhaps, we could give the new transformation an optional ivarmap argument, then it can put the ivar in the ivarmap if it did its thing (and not post the = intvar); then perhaps that is a safer approach where the solvers can later check the ivarmap? |
Starting from #835, allow for
decompose_customwhich is populated with a new transformation "decompose_linear" containing a few specialized decompositions.Only draft for now, and some open questions:
encoding == intvaror do we eliminate the intvar all together?bv == (iv == val)style constraints after flattening, and we don't do the encoding explicitely during decompose.