Gecode / gecode

Generic Constraint Development Environment
https://www.gecode.org
Other
275 stars 76 forks source link

Add support for the cumulatives constraint for MiniZinc #183

Closed cyderize closed 9 months ago

cyderize commented 10 months ago

MiniZinc 2.8 will have the cumulatives constraint in the globals library, so the full cumulatives propagator can now be used.

zayenz commented 10 months ago

Great that MiniZinc is getting exgended support for cumulatives.

Would it be possible to add some simple test of the variants that are posted, for example in test/flatzinc/cumulatives.cpp. As the posting code handles a lot of different cases, it is easy to mess that logic up and it would be good to see that all the code paths are exercised.

cyderize commented 9 months ago

I've added test cases for each of the code paths - I'm not sure how to actually make the tests check that the correct version is used though, since it would still actually work even if the dispatch always chose the var version every time.

I checked it manually and can see that every variant is used as expected though.

zayenz commented 9 months ago

Great!

Yes, the tests could still pass even if everything is passed on as variables, so in that sense they are not checking. But they do make sure that the logic does not try to get the value of an unassigned variable. The latter would crash a users program, while the former would just be a slight de-optimization.