Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the cumulatives constraint for MiniZinc #183

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

cyderize
Copy link

@cyderize cyderize commented Nov 7, 2023

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

@zayenz
Copy link
Member

zayenz commented Nov 10, 2023

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
Copy link
Author

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
Copy link
Member

zayenz commented Nov 13, 2023

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.

@zayenz zayenz merged commit 3a29c3e into Gecode:release/6.3.0 Nov 13, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants