Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore sequential argument when using optimize_acqf_mixed #2545

Closed
wants to merge 1 commit into from

Conversation

esantorella
Copy link
Contributor

Summary:
Passing sequential has been raising a deprecation warning, and stopping passing it enables pytorch/botorch#2390 .

Currently, the sequential argument is always provided by Ax and can be overridden by the user. This solution silently ignores the argument whenever the mixed optimizer is used. A nicer solution would be for Ax to only construct the sequential argument when it is needed and for there to be an exception when the user passes sequential=False and the mixed optimizer is used. If BoTorch plans to eventually enable sequential=True with optimize_acqf_mixed, then the exception should be raised by BoTorch so that Ax doesn't have to stay in sync with BoTorch's current capabilities. However, I think this code could use a thorough cleanup, so I went with the simple solution rather than add more if statements.

Differential Revision: D59057005

Summary:
Passing `sequential` has been raising a deprecation warning, and stopping passing it enables  pytorch/botorch#2390 .

Currently, the `sequential` argument is always provided by Ax and can be overridden by the user. This solution silently ignores the argument whenever the mixed optimizer is used. A nicer solution would be for Ax to only construct the `sequential` argument when it is needed and for there to be an exception when the user passes `sequential=False` and the mixed optimizer is used. If BoTorch plans to eventually enable `sequential=True` with `optimize_acqf_mixed`, then the exception should be raised by BoTorch so that Ax doesn't have to stay in sync with BoTorch's current capabilities. However, I think this code could use a thorough cleanup, so I went with the simple solution rather than add more `if` statements.

Differential Revision: D59057005
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 26, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D59057005

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.22%. Comparing base (8bc2c59) to head (b1b129c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2545   +/-   ##
=======================================
  Coverage   95.22%   95.22%           
=======================================
  Files         483      483           
  Lines       47220    47221    +1     
=======================================
+ Hits        44963    44964    +1     
  Misses       2257     2257           

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 18d0842.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants