search

pytorch / botorch

Bayesian optimization in PyTorch
https://botorch.org/
MIT License
3.11k stars 406 forks source link

Homotopy explicit all kwargs #2588

Closed CompRhys closed 1 month ago

CompRhys commented 1 month ago

This PR seeks to address: https://github.com/pytorch/botorch/issues/2579

optimize_acqf_homotopy is a fairly minimal wrapper around optimize_acqf but didn't have all the constraint functionality.

This PR copies over all of the arguments that we could in principal want to use up into optimize_acqf_homotopy. For the time being final_options has been kept. The apparent bug with fixed features not being passed to the final optimization has been fixed.

a simple dict rather than OptimizeAcqfInputs dataclass is used to store the shared parameters.

Related PRs

The original approach in https://github.com/pytorch/botorch/pull/2580 made use of kwargs which was opposed due to being less explicit.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.98%. Comparing base (ca956e1) to head (2761d54). Report is 8 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2588 +/- ## ======================================= Coverage 99.98% 99.98% ======================================= Files 196 196 Lines 17333 17345 +12 ======================================= + Hits 17330 17343 +13 + Misses 3 2 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

facebook-github-bot commented 1 month ago

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 month ago

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

CompRhys commented 1 month ago

Can we hold off merging this as I signed the CLA for a previous commit in my personal time and someone at my new company wants the lawyer to check

saitcakmak commented 1 month ago

Sure, let us know when it is safe to merge.

CompRhys commented 1 month ago

we're good to go

facebook-github-bot commented 1 month ago

@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 1 month ago

@saitcakmak merged this pull request in pytorch/botorch@24f659cb38ff4f9bb341d016082ac91656c6dac8.