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

Initial designs don't respect constraints #291

Open
apaleyes opened this issue Apr 8, 2020 · 7 comments
Open

Initial designs don't respect constraints #291

apaleyes opened this issue Apr 8, 2020 · 7 comments

Comments

@apaleyes
Copy link
Collaborator

apaleyes commented Apr 8, 2020

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.

@mmahsereci
Copy link
Contributor

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

@apaleyes
Copy link
Collaborator Author

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

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.

@mmahsereci
Copy link
Contributor

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.

@apaleyes
Copy link
Collaborator Author

apaleyes commented Apr 17, 2020

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.

@ntenenz
Copy link
Contributor

ntenenz commented May 22, 2021

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?

@apaleyes
Copy link
Collaborator Author

apaleyes commented May 24, 2021

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

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

No branches or pull requests

3 participants