Validate constraints in optimize_acqf #1231
Conversation
In some cases we may allow not using box constraints (e.g. when optimizing Alebo). Also, in general it would be good to the optimziation raise an error if the constraint set is empty.
facebook-github-bot
added
the
CLA Signed
Codecov Report
@@ Coverage Diff @@
## main #1231 +/- ##
===========================================
- Coverage 100.00% 99.99% -0.01%
===========================================
Files 122 122
Lines 10553 10607 +54
===========================================
+ Hits 10553 10606 +53
- Misses 0 1 +1
Continue to review full report at Codecov.
|
|
Not sure why the MacOS tests are failing, but that's unrelated to this PR. |
|
@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
One question here is whether we're ok with solving an LP on each call of |
This allows not running the validation. This is useful e.g in inner loops in order to avoid re-running the validation with the same parameters repeatedly.
|
@Balandat has updated the pull request. You must reimport the pull request before landing. |
|
@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Coverage gap is unrelated and fixed in #1234 |
Balandat
changed the title
| # min_(x, s) - sum_i(s_i) | ||
| # s.t. -x <= s <= x |
There was a problem hiding this comment.
I don't think this is correct - we're trying to solve
max \sum_i |x_i|
and the correct transformation would be
max \sum_i s_i
s_i <= |x_i|
The constraint |x_i| >= s_i is equal to (x_i >= s_i) OR (x_i <= -s_i) and is not convex and so won't be representable as an LP.
The constraint given here, -x _i<= s_i <= x_i is equivalent to |s_i| <= x, which is convex but is not correct for the problem we're trying to solve. In particular it will only provide the correct answer for positive values of x_i; if the inequality and equality constraints are unbounded towards -Inf but has volume in the positive orthant, this LP will not detect the unboundedness.
…specified In some cases we may allow not using box constraints (e.g. when optimizing Alebo). This removes a check that currently breaks Alebo optimization. A proper solution should actually validate the constraint set, this was started in pytorch#1231 but there are some nontrivial issues with this, so this provides a quick fix in the meantime.
…specified (#1297) Summary: In some cases we may allow not using box constraints (e.g. when optimizing Alebo). This removes a check that currently breaks Alebo optimization. A proper solution should actually validate the constraint set, this was started in #1231 but there are some nontrivial issues with this, so this provides a quick fix in the meantime. Pull Request resolved: #1297 Reviewed By: saitcakmak Differential Revision: D37773082 Pulled By: Balandat fbshipit-source-id: cc64868a6c0af61aa3c846d2ed51da02cc5f6a20
|
Is there still a need for this? Conceptually the |
|
Yeah I think we still want to do this, just correctly :) I think we should be able to use a slightly different reformulation of the problem to avoid the non-convexity issue. |
In some cases we may allow not using box constraints (e.g. when optimizing Alebo). Also, in general it would be good to the optimziation raise an error if the constraint set is empty.
Will require some unit tests.