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

[Feature Request] Add maximize flag to PosteriorMean acquisition function #875

Closed
lukasfro opened this issue Jul 22, 2021 · 6 comments
Closed
Labels
enhancement New feature or request

Comments

@lukasfro
Copy link
Contributor

Unlike other acquisition functions, the PosteriorMean does not have a maximize flag, hence it is not currently not possible to find the minimum of the posterior mean function via optimize_acqf. This should not be too much effort to implement. Happy to create a PR when the feature is desired.

@lukasfro lukasfro added the enhancement New feature or request label Jul 22, 2021
@saitcakmak
Copy link
Contributor

Hi @lukasfro. With the existing API, you can use

pm = PosteriorMean(model=model, objective=ScalarizedObjective(weights=torch.tensor([-1.0]))
optimize_acqf(...)

to minimize the posterior mean. This will internally negate the posterior, so any value you get out of this will also be negated.

You may have noticed that everything is maximized by default in BoTorch. If you need to minimize a function, it is typically easiest to just negate your function evaluations and maximize that. In the past, I worked with BoTorch in a minimization setting without negating the function evaluations and things get messy pretty quickly.

@lukasfro
Copy link
Contributor Author

Hi @saitcakmak, thanks for the clarification. It looks like ScalarizedObjective is the way to go then. I thought it might be nice to have a consistent interface to the other analytical acquisition functions that do make use of the maximize flag.

@Balandat
Copy link
Contributor

I think it would be fine to add a maximize arg to PosteriorMean for consistency with the other analytic acquisition functions. Should be trivial to do by adding a constructor and flipping a sign conditionally on that in forward. Happy to review a PR for this :)

@lukasfro
Copy link
Contributor Author

Sounds good to me. Will create the PR next week!
I just noticed that ScalarizedPosteriorMean could be extended in a similar manner. Should this be included in the PR as well?

@lukasfro lukasfro reopened this Jul 23, 2021
@Balandat
Copy link
Contributor

I don't think ScalarizedPosteriorMean needs a maximize option, since this can easily be done just by flipping the sign of the weights.

@Balandat
Copy link
Contributor

Balandat commented Dec 8, 2021

For PosteriorMean, this was added by #881.

@Balandat Balandat closed this as completed Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants