emukit icon indicating copy to clipboard operation
emukit copied to clipboard

Initial designs don't respect constraints

Open apaleyes opened this issue 4 years ago • 7 comments

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.

apaleyes avatar Apr 08 '20 01:04 apaleyes

Good catch, Andrei, thanks! until this is solved a comment in the docstring might help. What do you think?

mmahsereci avatar Apr 14 '20 14:04 mmahsereci

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?

apaleyes avatar Apr 14 '20 20:04 apaleyes

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:

  1. 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.
  2. Generate a list, test points in there, and only return those that comply.
  3. 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.

apaleyes avatar Apr 14 '20 20:04 apaleyes

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.

mmahsereci avatar Apr 17 '20 17:04 mmahsereci

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.

apaleyes avatar Apr 17 '20 17:04 apaleyes

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 avatar May 22 '21 15:05 ntenenz

@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.

apaleyes avatar May 24 '21 10:05 apaleyes