-
Notifications
You must be signed in to change notification settings - Fork 554
[WIP] Evaluate random point instead of using point twice #1050
base: master
Are you sure you want to change the base?
[WIP] Evaluate random point instead of using point twice #1050
Conversation
Instead of just warning the user about evaluating a point twice, use a new random point instead (and include this information in the warning)
| min_delta_x = min([self.space.distance(next_x, xi) | ||
| for xi in self.Xi]) | ||
| if abs(min_delta_x) <= 1e-8: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, the next best point should be evaluated instead, but I do not know how to do that.
I guess you would have to move this check to where self._next_x is set and recompute it there.
|
Since there are use case for both the old and the new behaviours, I would argue for a flag in the optimizer's |
@QuentinSoubeyran What is the alleged use case for this old behavior? |
@kernc This is what I was referring to. If such use cases are rare, defaulting to the new behaviour might be better, now that I think about it. |
When a duplicate point is proposed as the next candidate, test if one of the acquisition functions proposed a candidate that has not been used yet. If yes, use that point instead. If no, fall back to a random point and warn about it.
|
It has been a while since my first commit to this PR. I realized that this issue is still not resolved and I took another stab at it. This increases the chance of replacing a duplicate candidate with a useful candidate a lot. It still cannot prevent using random points as a fallback. I still do not know if it is possible to force the acquisition functions themselves to not create duplicates in the first place. I included a bool |
#302 discusses a long standing issue where the ask() function to "Query point or multiple points at which objective should be evaluated" will just repeatedly return the same point(s) to evaluate. In cases where the objective always returns the same results, the solver can even get stuck in a loop.
This PR aims to mitigate the issue. Instead of just warning the user about evaluating a point twice, use a new random point instead (and include this information in the warning).
This is not a good solution. Ideally, the next best point should be evaluated instead, but I do not know how to do that.
Also, in #302 a user said "Usually evaluating the same point multiple times isn't something you want to do, but if you have a (very) noisy objective it makes sense to do so." So the solution presented here might actually cause a problem for users who want the same point to be evaluated multiple times. Since this is up for debate, I am tagging it as [WIP], even though I think it is "complete" in the way that I am intending it to be.
Maybe this just stays here as a reference for users who want a very easy workaround that they can implement themselves.