emukit
emukit copied to clipboard
Initial designs don't respect constraints
It seems that none of the initial designs checks for constraints.
It isn't obvious how to fix it though. For random things are pretty clear: consider points individually, and just reject and resample until you give up or the necessary amount is collected. Not sure if the same approach applies to Latin and Sobol.
Good catch, Andrei, thanks! until this is solved a comment in the docstring might help. What do you think?
I don't think such bugs belong to the docs. We should just fix it right away. Any thoughts on how to approach sobol and latin?
To clarify why this is complicated: it appears that neither method really can be done in incremental fashion. So unlike with random design, it doesn't seem possible to just dismiss a point and add a new one.
Some initial thoughts:
- Generate a list of samples, check if they all comply, if yes return, if not repeat from the beginning. Give up with error after N tries.
- Generate a list, test points in there, and only return those that comply.
- Generate a list, dismiss all non-compliant, and generate replacements with RandomDesign
All three have their flaws, so I am not advocating for any of these atm. This is just to get the conversation going.
Hi Andrei, if the bug can be fixed right away this is even better. If not at least we can point it out in the docstring e.g., "This initial design cannot be used with a parameter space with constraints". As a user I would appreciate that before having to figure out why my simulation crashes because it is evaluated on bad points.
That is actually a good idea, to rule out the bug quickly while we are thinking about how to respect the constraints - just throw a quick exception.
Resurrecting this conversation since I just ran into the same issue. I'd be happy to contribute a small PR to at least solve this for RandomDesign
. A proposal:
- Add an argument to
RandomDesign.__init__
to enable resampling if constraints are not satisfied (default to False to preserve backwards compatibility) - Add an argument to set an upper bound on the number of resamples to perform (default to -1 to retry until successful)
- On retry, retain valid samples and resample to fill those missing
- If resampling is enabled and the threshold is met without generating enough valid samples, raise an exception.
Thoughts?
@ntenenz that sounds wonderful, please go ahead with the PR. All your suggestions sound fair, except that I don't believe we need to be backwards compatible. This is just a bug that needs fixing, not a feature that is chaning behavior.