Skip to content
This repository was archived by the owner on Feb 28, 2024. It is now read-only.

Conversation

@jnettels
Copy link

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

Instead of just warning the user about evaluating a point twice, use a new random point instead (and include this information in the warning)
Comment on lines 446 to 448
min_delta_x = min([self.space.distance(next_x, xi)
for xi in self.Xi])
if abs(min_delta_x) <= 1e-8:
Copy link
Contributor

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.

@QuentinSoubeyran
Copy link
Contributor

Since there are use case for both the old and the new behaviours, I would argue for a flag in the optimizer's __init__ that defaults to the old behaviour. This also preserve backward compatibility.

@kernc
Copy link
Contributor

kernc commented Sep 22, 2021

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.

@QuentinSoubeyran What is the alleged use case for this old behavior?

@QuentinSoubeyran
Copy link
Contributor

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

@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.
@jnettels
Copy link
Author

jnettels commented Jun 28, 2023

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.
I also realized that we have access to the proposed next point candidates from all acquisition functions, when gp_hedge is used. So my code now does the following:
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.

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.
But nevertheless this increases the speed of reaching a solution for some of my tests.

I included a bool avoid_duplicates that could be used (by someone else?) to somehow make this behaviour selectable as an option for the user, but I don't know where to best put that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants