Optimization over mixed spaces in `optimize_acqf_homotopy` #2639

jduerholt · 2024-11-27T13:51:13Z

Motivation

So far, optimize_acqf_homotopy can only handle fixed_features but not fixed_features_list which is useful for optimization over mixed spaces. In the spirit of optimize_acqf_list, this PR adds the option for fixed_features_list by using optimize_acqf_mixed instead of optimize_acqf in optimize_acqf_homotopy, when fixed_features_list is provided.

Currently, it is not working as optimize_acqf_mixed has no option return_best_only=False like optimize_acqf. The easiest way to solve it (maybe not the best), would be to provide an option to return the candidates from all restarts for the finally picked fixed features combination in optimize_acqf_mixed. For the start, it would be sufficient to only implement it for q=1, and error out for the case of q>1.

What do you think?

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Unit tests.

Balandat · 2024-11-27T17:28:23Z

Currently, it is not working as optimize_acqf_mixed has no option return_best_only=False like optimize_acqf. The easiest way to solve it (maybe not the best), would be to provide an option to return the candidates from all restarts for the finally picked fixed features combination in optimize_acqf_mixed. For the start, it would be sufficient to only implement it for q=1, and error out for the case of q>1. What do you think?

I think that makes sense. Starting with only supporting the q=1 case seems very reasonable (let's add the idea above in the comments though where we raise that UnsupportedError).

Balandat · 2024-11-27T17:23:50Z

botorch/optim/optimize_homotopy.py

+    if fixed_features and fixed_features_list:
+        raise ValueError(
+            "Èither `fixed_feature` or `fixed_features_list` can be provided, not both."
+        )


fixed_features as an arg seems redundant if we have fixed_features_list. We should probably deprecate the fixed_features arg and raise a warning when passed. And if fixed_features_list contains only one element we can just use that with optimize_acqf

Raise a warning but still give the user the possibility to pass it for now? Should the same then also be done for optimize_acqf_list? There the same setup is used.

yes, that would be great

I implemeted it now for optimize_acqf_homotopy, if it is fine for you how it looks there, I will port it to optimize_acqf_list.

test/optim/test_homotopy.py

Co-authored-by: Max Balandat <[email protected]>

jduerholt · 2024-11-28T11:29:01Z

I added the possibility return_best_only=False for q=1 in optimize_acqf_mixed and adapted the tests. Also optimize_acqf_homotopy works now with a fixed_features_list. For this, I had to remove the following options from optimize_acqf_homotopy as they are not available for optimize_acqf_mixed:

gen_candidates: would it make sense for you to add this option to optimize_acqf_mixed?
sequential: this keyword make for me no sense in the kontext of optimize_homotopy, as we are always using q=1 for the calls to optimize_acqf
return_full_tree: I am not sure about this as I do not exactly know what it does. Does it make sense in kontext of optimize_homotopy?

What do you think?

Balandat

gen_candidates: would it make sense for you to add this option to optimize_acqf_mixed?

Yeah I think that would make sense. Inside of optimize_acqf_mixed we'd just pass this to optimize_acqf.

sequential: this keyword make for me no sense in the kontext of optimize_homotopy, as we are always using q=1 for the calls to optimize_acqf

True - I think we can remove this from optimize_acqf_homotopy.

return_full_tree: I am not sure about this as I do not exactly know what it does. Does it make sense in kontext of optimize_homotopy?

This is used by lookahead acquisition functions. I don't think we need to support this for optimize_acqf_homotopy.

botorch/optim/optimize.py

botorch/optim/optimize_homotopy.py

Balandat · 2024-11-28T23:05:38Z

botorch/optim/optimize_homotopy.py

+    if fixed_features and fixed_features_list:
+        raise ValueError(
+            "Èither `fixed_feature` or `fixed_features_list` can be provided, not both."
+        )


yes, that would be great

botorch/optim/optimize_homotopy.py

Co-authored-by: Max Balandat <[email protected]>

Balandat

Thanks - largely nits except for the conditional when "Optimize one more time with the final options".

Also looks like you have a KeyError test failure as well as line length lint issue

botorch/optim/optimize.py

botorch/optim/optimize_homotopy.py

test/optim/test_homotopy.py

test/optim/test_optimize.py

jduerholt · 2024-12-02T15:33:57Z

Hi @Balandat,

thanks for your review(s). I think, I have now implemented all the suggestions.

Could you have a look on the issue that I always have to wait for approval before the pipelines start? In the past, they were starting always automatically which helped me to faster finish the PRs ;) And maybe it is also less annoying for you ...

Best,

Johannes

Balandat · 2024-12-02T16:01:04Z

Could you have a look on the issue that I always have to wait for approval before the pipelines start? In the past, they were starting always automatically which helped me to faster finish the PRs ;) And maybe it is also less annoying for you ...

Yeah I pinged some folks to understand if we can avoid this, let's see what they say.

codecov · 2024-12-02T16:10:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.98%. Comparing base (4190f74) to head (1c53e3d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2639   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         196      196           
  Lines       17410    17431   +21     
=======================================
+ Hits        17408    17429   +21     
  Misses          2        2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Balandat · 2024-12-02T16:44:19Z

Looks like you're missing a unit test for more than 1 elements in the fixed_features_list which seems important :)

jduerholt · 2024-12-02T19:54:25Z

Looks like you're missing a unit test for more than 1 elements in the fixed_features_list which seems important :)

Ah, this is now a consequence of the new structure where we call optimize_acqf when fixed_features_list has only one element. Before it was also in this case going to òptimize_acqf_mixed. I will add a new test case.

jduerholt · 2024-12-02T20:12:15Z

Now, the test is there ;), But there is still something wrong. Need to fix it ;)

jduerholt · 2024-12-02T20:34:41Z

But there is still something wrong. Need to fix it ;)

Not it is actually correct, I was overlooking something. Too late for coding ;)

Balandat

Tutorial failure is unrelated (and will be fixed in #2641). Thanks!

facebook-github-bot · 2024-12-02T21:26:32Z

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

esantorella · 2024-12-02T21:45:45Z

botorch/optim/optimize.py

@@ -956,15 +979,18 @@ def optimize_acqf_mixed(
                    post_processing_func=post_processing_func,
                    batch_initial_conditions=batch_initial_conditions,
                    ic_generator=ic_generator,
-                    return_best_only=True,
+                    return_best_only=False,


Why not return_best_only=return_best_only? (A comment would be helpful here)

We get here all candidates and then filter later based on the return_best_only argument. I will add a comment.

esantorella · 2024-12-02T21:46:58Z

botorch/optim/optimize.py

            ic_generator=ic_generator,
            ic_gen_kwargs=ic_gen_kwargs,
+            timeout_sec=timeout_sec,
+            retry_on_optimization_warning=retry_on_optimization_warning,
+            return_best_only=True,


Why True here and False earlier?

Because, currently return_best_only is only implemented for q=1 and here we are in the loop for q>1.

test/optim/test_homotopy.py

test/optim/test_optimize.py

jduerholt · 2024-12-03T07:59:59Z

Thanks for the review @esantorella. I adressed your comments and resolved the merge conflicts with main. Best, Johannes

facebook-github-bot · 2024-12-03T14:59:50Z

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

facebook-github-bot · 2024-12-03T18:48:15Z

@Balandat merged this pull request in 88f47bc.

add optimize_acqf_mixed

1e5325e

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Nov 27, 2024

Balandat reviewed Nov 27, 2024

View reviewed changes

jduerholt mentioned this pull request Nov 27, 2024

Add Trust Regions experimental-design/bofire#457

Draft

2 tasks

jduerholt and others added 2 commits November 28, 2024 08:32

Update test/optim/test_homotopy.py

cd55187

Co-authored-by: Max Balandat <[email protected]>

make it working

3baaad4

Balandat reviewed Nov 28, 2024

View reviewed changes

jduerholt and others added 3 commits November 29, 2024 11:35

Update botorch/optim/optimize.py

501c19c

Co-authored-by: Max Balandat <[email protected]>

Update botorch/optim/optimize.py

02f4920

Co-authored-by: Max Balandat <[email protected]>

implement suggestions

c873849

Balandat reviewed Dec 2, 2024

View reviewed changes

jduerholt added 3 commits December 2, 2024 15:31

fix test

a72da7d

add warning

a059ca8

implement suggestions

d5a4d4a

Balandat closed this Dec 2, 2024

Balandat reopened this Dec 2, 2024

add missing test

884f918

Balandat approved these changes Dec 2, 2024

View reviewed changes

esantorella reviewed Dec 2, 2024

View reviewed changes

test/optim/test_homotopy.py Show resolved Hide resolved

esantorella reviewed Dec 2, 2024

View reviewed changes

test/optim/test_optimize.py Outdated Show resolved Hide resolved

esantorella reviewed Dec 2, 2024

View reviewed changes

test/optim/test_optimize.py Outdated Show resolved Hide resolved

esantorella reviewed Dec 2, 2024

View reviewed changes

test/optim/test_optimize.py Outdated Show resolved Hide resolved

jduerholt added 2 commits December 3, 2024 08:26

implement suggestions

aa1e22a

Merge branch 'main' into feature/homotopy_fixed_features_list

1c53e3d

facebook-github-bot closed this in 88f47bc Dec 3, 2024

facebook-github-bot added the Merged label Dec 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimization over mixed spaces in `optimize_acqf_homotopy` #2639

Optimization over mixed spaces in `optimize_acqf_homotopy` #2639

jduerholt commented Nov 27, 2024

Balandat commented Nov 27, 2024

Balandat Nov 27, 2024

jduerholt Nov 28, 2024

Balandat Nov 28, 2024

jduerholt Dec 2, 2024

jduerholt commented Nov 28, 2024

Balandat left a comment

Balandat Nov 28, 2024

Balandat left a comment

jduerholt commented Dec 2, 2024 •

edited

Loading

Balandat commented Dec 2, 2024

codecov bot commented Dec 2, 2024 •

edited

Loading

Balandat commented Dec 2, 2024

jduerholt commented Dec 2, 2024

jduerholt commented Dec 2, 2024 •

edited

Loading

jduerholt commented Dec 2, 2024

Balandat left a comment

facebook-github-bot commented Dec 2, 2024

esantorella Dec 2, 2024 •

edited

Loading

jduerholt Dec 3, 2024

esantorella Dec 2, 2024

jduerholt Dec 3, 2024

jduerholt commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

Optimization over mixed spaces in optimize_acqf_homotopy #2639

Optimization over mixed spaces in optimize_acqf_homotopy #2639

Conversation

jduerholt commented Nov 27, 2024

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Balandat commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jduerholt commented Nov 28, 2024

Balandat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Balandat left a comment

Choose a reason for hiding this comment

jduerholt commented Dec 2, 2024 • edited Loading

Balandat commented Dec 2, 2024

codecov bot commented Dec 2, 2024 • edited Loading

Codecov Report

Balandat commented Dec 2, 2024

jduerholt commented Dec 2, 2024

jduerholt commented Dec 2, 2024 • edited Loading

jduerholt commented Dec 2, 2024

Balandat left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Dec 2, 2024

esantorella Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jduerholt commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

Optimization over mixed spaces in `optimize_acqf_homotopy` #2639

Optimization over mixed spaces in `optimize_acqf_homotopy` #2639

jduerholt commented Dec 2, 2024 •

edited

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading

jduerholt commented Dec 2, 2024 •

edited

Loading

esantorella Dec 2, 2024 •

edited

Loading