search

Gecode / gecode

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

Add RestartStop #123

Closed Dekker1 closed 3 years ago

Dekker1 commented 3 years ago

This PR adds the RestartStop class that checks whether a given cutoff for the number of restarts is reached. The RestartStop also made part of CombinedStop.

I had to construct a stopping mechanism based on the restart count for an experiment and it seems like a useful tool in general. I think I managed to patch all the locations correctly, but I maybe someone with more experience could find a place I missed.

Also a question for an expert: Currently the stop is setup to function the same as FailStop, but I notice that the statistics always report 1 more than the set limit. (This is now true for both FailStop and RestartStop). Is an issue in the statistics or if these stops actually be using a >= comparison?

zayenz commented 3 years ago

Thanks @Dekker1, this looks very reasonable and like a nice addition.

One thing that we need to do is also adding this to MPG.

Dekker1 commented 3 years ago

Something I did notice today is that the RBS Engine also has a Stop class called RestartStop. It works very differently (as it seems to determine whether the search should be restarted), but I'm wondering about the name conflict. The other occurrence is in a different namespace, so there is no direct conflict, but I was wondering if that is okay, or if I should use a different name.

zayenz commented 3 years ago

I think the name in the new context is very reasonable, and is the name I would expect it to have. Thus I would like to keep it the same.

The name in the RBS engine is a bit misleading, since it as you say does other things than just stop based on when a restart count is reached. I would actually like to rename it, but that might be a breaking change for some since it is an exported symbol.

As the names are in different name spaces, I think we can merge it as is for now. Will wait a bit before merging though to consider the naming issue a bit more.