Multi dimensional element constraint#926
Conversation
IgnaceBleukx
left a comment
There was a problem hiding this comment.
Great! I have some (mostly minor) comments on the code, but the CI seems to be failing for PySAT, not sure why...
Other thing is the name, personally I like MultiDElement, what do you think?
|
Fixed the issue with pysat, it was a memory thing, just reduced the size of the array in the test and done. Also resolved the comments, improving based on the suggestions, and adding additional tests. About the name, I do not have any strong preference, will change it to MultiDElement. |
IgnaceBleukx
left a comment
There was a problem hiding this comment.
Great! Not sure if the helper function is really required?
Also, MultiD element is also supported in MiniZinc, right? So we should also add it to our interface.
I quickly checked for other solvers, they don't seem to support it.
|
NDElement, like we have NDVarArray? (no real objection to MultiDElement though) |
|
Interesting, is this with gecode or OR-Tools as backend? I would still prefer to not decompose it, as users may want to use a solver with minizinc that does support native 2d element constraints. |
|
Renamed to NDElement, and merged current master into this one. |
tias
left a comment
There was a problem hiding this comment.
I added some stricter typecheck to the constructor; well, an assert actually, and had to update a test.
our safen has a 'safen_toplevel' that multiple solvers have to fill in with 'element', 'div', mod'... should we also add nd_element there?
More generally, safening docs don't mention nd_element yet
we now have a decompose_linear for element, do we also need one for nd_element?
I also followed an agent recommendation to extend a test...
also small bugfix in choco's added objective safening
other than that its looking good so far...

Adding Multi-Dimensional Element constraint separately than 1D Element. The main reason is to allow us to safen the indices, as the way we have it now we lose information for the different indices.