Homotopy explicit all kwargs #2588
Conversation
facebook-github-bot
added
the
CLA Signed
CompRhys
force-pushed
the
homotopy-explicit-all-kwargs
branch
from
df8ffad
to
81320a2
Compare
CompRhys
force-pushed
the
homotopy-explicit-all-kwargs
branch
from
81320a2
to
b3e8aaf
Compare
botorch/optim/optimize_homotopy.py
Outdated
| from collections.abc import Callable | ||
|
|
||
| from botorch.generation.gen import TGenCandidates | ||
| from botorch.optim.initializers import ( | ||
| TGenInitialConditions, | ||
| ) |
There was a problem hiding this comment.
Could you run ufmt format . on the PR? This will fail the linter.
There was a problem hiding this comment.
Sure, Can I add a pre-commit yaml to have this be an auto hook? doesn't work out of the box with ufmt
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2588 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 196 196
Lines 17333 17345 +12
=======================================
+ Hits 17330 17343 +13
+ Misses 3 2 -1 ☔ View full report in Codecov by Sentry. |
|
@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
test/optim/test_homotopy.py
Outdated
| inequality_constraints=constraints, | ||
| ) | ||
| self.assertEqual(candidate.shape, torch.Size([1, 2])) | ||
| self.assertGreaterEqual(candidate.sum(), 2.0) |
There was a problem hiding this comment.
| self.assertGreaterEqual(candidate.sum(), 2.0) | |
| self.assertGreaterEqual(candidate.sum().item(), 2.0) |
There was a problem hiding this comment.
Weird that this is failing. I was gonna try comparing floats but looks like I don't have commit access to the PR
There was a problem hiding this comment.
If I make this change it at least makes the failure explict:
FAILED test/optim/test_homotopy.py::TestHomotopy::test_optimize_acqf_homotopy - AssertionError: 1.999999999999982 not greater than or equal to 2.0Would you be okay if I change the test to >= 2.0 - 1e-6?
There was a problem hiding this comment.
That sounds reasonable to me
… floating point errors
|
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 |
|
Sure, let us know when it is safe to merge. |
|
we're good to go |
|
@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@saitcakmak merged this pull request in 24f659c. |
This PR seeks to address: #2579
optimize_acqf_homotopyis a fairly minimal wrapper aroundoptimize_acqfbut 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 beingfinal_optionshas been kept. The apparent bug with fixed features not being passed to the final optimization has been fixed.a simple dict rather than
OptimizeAcqfInputsdataclass is used to store the shared parameters.Related PRs
The original approach in #2580 made use of kwargs which was opposed due to being less explicit.