Buxfix for Proximal acquisition function wrapper for negative base acquisition functions #1447

roussel-ryan · 2022-10-10T19:23:02Z

Motivation

This PR fixes a major issue when using the ProximalAcquisitionFunction with base acquisition functions that are not strictly positive. This PR fixes it by applying a Softplus transformation to the base acquisition function values (using optional beta = 1,0 argument) before multiplying by proximal weighting.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Tests have been updated with correct (softplus transformed) values.

…to solve issue with qExpectedHypervolumeImprovement

codecov · 2022-10-10T19:27:35Z

Codecov Report

Merging #1447 (5a6ceaf) into main (0277720) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 5a6ceaf differs from pull request most recent head 213be32. Consider uploading reports for the commit 213be32 to get more accurate results

@@            Coverage Diff            @@
##              main     #1447   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          124       124           
  Lines        11548     11554    +6     
=========================================
+ Hits         11548     11554    +6

Impacted Files	Coverage Δ
botorch/acquisition/proximal.py	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

saitcakmak

Thanks, lgtm! The tutorial failure is unrelated (#1446), so please ignore it.

facebook-github-bot · 2022-10-10T19:49:26Z

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

SebastianAment · 2022-10-10T20:36:40Z

This is a higher level design question, rather than a comment on the particular fix.

@roussel-ryan, is there a reason against simply adding a quadratic to an arbitrary acquisition function? With this fix, ProximalAcquisitionFunction will compute

log(1+exp(acquisition(x))) * exp(-Q(x)),

where Q(x) = (x-x_{n-1}).t() * proximal_weights^{-1} * (x-x_{n-1})). Similar biasing could be achieved with the simpler expression

acquisition(x) - Q(x).

In addition, the latter would not change the relative magnitude of the acquisition function to the proximal term if the acquisition is negative. If it takes a large negative value, softplus might also lead to numerical instabilities.
Would love to hear your thoughts!

Balandat · 2022-10-10T21:03:01Z

Thanks for fixing this. One thing that we should keep in mind here is that if the acquisition function values are small (as is often the case later during the optimization for things like Expected Improvement) this could substantially impact the behavior for functions that are indeed guaranteed to be positive. So in that case it seems that the choice of a large beta would be necessary (and in general this highlights the importance of the choice of beta). Would it make sense to allow passing in beta as optional and fall back to the current behavior?

roussel-ryan · 2022-10-10T21:13:20Z

@SebastianAment In response to large negative values for the base acquisition function I'm hoping that using a standardize outcome transform will prevent extreme values. It's not a perfect solution but I think for most cases the proposed solution is sufficient. As for adding the biasing, instead of multiplying it you mentioned that the relative scale of each term would remain the same, but I think it presents the biasing from working when the acquisition function is large (or vice versa). Unless I'm misunderstanding what you wrote?

roussel-ryan · 2022-10-10T21:19:43Z

@Balandat I could potentially remove the transform when all of the acquisition function values are found to be positive. Alternatively I could set a large beta as the default or raise an error when the acqf values are negative. We need to prevent it from ever being used with negative acqf because the issue can be quite easy to overlook

SebastianAment · 2022-10-10T21:37:12Z

As for adding the biasing, instead of multiplying it you mentioned that the relative scale of each term would remain the same, but I think it presents the biasing from working when the acquisition function is large (or vice versa)

That makes sense. The multiplicative version lets us choose weights proportional to the scale of the acquisition functions, which we might not know a priori but could update during a run. It's also equivalent to the additive version by taking the log of the acquisition function, and that might be more easily applied to negative acquisition values.
Thanks for the quick response, enjoyed reading your paper!

Balandat · 2022-10-16T04:09:07Z

I am not sure how widely used this is. The main thing I'm worried about is that the default behavior for non-negative but small-in-magnitude acquisition functions may change quite a bit with this change. How about we (i) make beta optional and if None do not apply the softplus, and (ii) raise an error if any of the acquisition values are negative, pointing the user to set a beta value?

…qf values

roussel-ryan · 2022-10-17T15:12:05Z

@Balandat made the change you suggested

Balandat

Thanks! A couple of nits, o/w this lgtm.

Balandat · 2022-10-18T04:28:10Z

botorch/acquisition/proximal.py

@@ -49,6 +53,7 @@ def __init__(
        acq_function: AcquisitionFunction,
        proximal_weights: Tensor,
        transformed_weighting: bool = True,
+        beta: float = None,


Suggested change

beta: float = None,

beta: Optional[float] = None,

Balandat · 2022-10-18T04:31:07Z

test/acquisition/test_proximal.py

+
+                    ei_prox_beta = EI_prox_beta(test_X)
+                    self.assertTrue(torch.allclose(ei_prox_beta, ei * test_prox_weight))
+                    self.assertTrue(ei_prox_beta.shape == torch.Size([1]))


nit

Suggested change

self.assertTrue(ei_prox_beta.shape == torch.Size([1]))

self.assertEqual(ei_prox_beta.shape, torch.Size([1]))

Balandat · 2022-10-18T04:32:42Z

test/acquisition/test_proximal.py

+                    test_prox_weight = torch.exp(
+                        mv_normal.log_prob(proximal_test_X)
+                    ) / torch.exp(mv_normal.log_prob(last_X))


Suggested change

test_prox_weight = torch.exp(

mv_normal.log_prob(proximal_test_X)

) / torch.exp(mv_normal.log_prob(last_X))

test_prox_weight = torch.exp(

mv_normal.log_prob(proximal_test_X) -

mv_normal.log_prob(last_X)

)

Balandat · 2022-10-18T04:33:39Z

botorch/acquisition/proximal.py

+    acquisition function. Then the acquisition function is
+    weighted via a squared exponential centered at the last training point,


Suggested change

acquisition function. Then the acquisition function is

weighted via a squared exponential centered at the last training point,

acquisition function. The acquisition function is

weighted via a squared exponential centered at the last training point,

Balandat · 2022-10-18T04:33:51Z

botorch/acquisition/proximal.py

+    weighted via a squared exponential centered at the last training point,
+    with varying lengthscales corresponding to `proximal_weights`. Can only be used
+    with acquisition functions based on single batch models. Acquisition functions
+    must be positive or `beta` is specified to apply a SoftPlus transform before


Suggested change

must be positive or `beta` is specified to apply a SoftPlus transform before

must be positive or `beta` must be specified to apply a SoftPlus transform before

Balandat · 2022-10-18T04:33:54Z

test/acquisition/test_proximal.py

+                    with self.assertRaises(RuntimeError):
+                        bad_neg_prox(test_X)


Can you use assertRaisesRegex in order to check for the proper error message here?

roussel-ryan · 2022-10-18T14:36:37Z

Updated w/suggestions. Thanks for the comments!

Balandat

No problem, thanks for the contribution!

@saitcakmak I think this is ready for a re-import and merge!

facebook-github-bot · 2022-10-18T15:39:56Z

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

roussel-ryan and others added 12 commits May 10, 2022 14:45

Add support for multi-output models in proximal biasing

1f07719

Merge branch 'main' of https://github.com/roussel-ryan/botorch

4be393d

removing excess code added by merge

b8116ad

Bugfix for specifying proximal weights in a multi-dim tensor

79fd58e

Fixed bug with X_pending uses

fe8e3bc

Copy base acquisition function X_pending to proximal class X_pending …

ee0bfaf

…to solve issue with qExpectedHypervolumeImprovement

Formatting

856f292

Fixed type/device issues, removed ProximalWarning

be35ff6

Merge branch 'main' of https://github.com/roussel-ryan/botorch

ee0f090

Add softplus transformation before proximal biasing

5bc54f8

Update test_proximal.py

4b10a99

Update proximal.py

da3acd7

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 10, 2022

saitcakmak approved these changes Oct 10, 2022

View reviewed changes

Merge branch 'main' into main

a3168ec

roussel-ryan added 2 commits October 17, 2022 10:06

Update to make beta specification optional and raise error for bad ac…

86e8945

…qf values

Updated docstrings

5a6ceaf

Balandat reviewed Oct 18, 2022

View reviewed changes

updates to fix small issues

213be32

Balandat approved these changes Oct 18, 2022

View reviewed changes

facebook-github-bot closed this in 98503e4 Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Buxfix for Proximal acquisition function wrapper for negative base acquisition functions #1447

Buxfix for Proximal acquisition function wrapper for negative base acquisition functions #1447

roussel-ryan commented Oct 10, 2022

codecov bot commented Oct 10, 2022 •

edited

Loading

saitcakmak left a comment

facebook-github-bot commented Oct 10, 2022

SebastianAment commented Oct 10, 2022 •

edited

Loading

Balandat commented Oct 10, 2022

roussel-ryan commented Oct 10, 2022

roussel-ryan commented Oct 10, 2022

SebastianAment commented Oct 10, 2022

Balandat commented Oct 16, 2022

roussel-ryan commented Oct 17, 2022

Balandat left a comment

Balandat Oct 18, 2022

Balandat Oct 18, 2022

Balandat Oct 18, 2022

Balandat Oct 18, 2022

Balandat Oct 18, 2022

Balandat Oct 18, 2022

roussel-ryan commented Oct 18, 2022

Balandat left a comment

facebook-github-bot commented Oct 18, 2022

	self.assertTrue(ei_prox_beta.shape == torch.Size([1]))
	self.assertEqual(ei_prox_beta.shape, torch.Size([1]))

		acquisition function. Then the acquisition function is
		weighted via a squared exponential centered at the last training point,

	must be positive or `beta` is specified to apply a SoftPlus transform before
	must be positive or `beta` must be specified to apply a SoftPlus transform before

Buxfix for Proximal acquisition function wrapper for negative base acquisition functions #1447

Buxfix for Proximal acquisition function wrapper for negative base acquisition functions #1447

Conversation

roussel-ryan commented Oct 10, 2022

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

codecov bot commented Oct 10, 2022 • edited Loading

Codecov Report

saitcakmak left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 10, 2022

SebastianAment commented Oct 10, 2022 • edited Loading

Balandat commented Oct 10, 2022

roussel-ryan commented Oct 10, 2022

roussel-ryan commented Oct 10, 2022

SebastianAment commented Oct 10, 2022

Balandat commented Oct 16, 2022

roussel-ryan commented Oct 17, 2022

Balandat left a comment

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

Balandat Oct 18, 2022

Choose a reason for hiding this comment

roussel-ryan commented Oct 18, 2022

Balandat left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 18, 2022

codecov bot commented Oct 10, 2022 •

edited

Loading

SebastianAment commented Oct 10, 2022 •

edited

Loading