Warn if inoperable keyword arguments are passed to optimizers #1677
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D43277102 |
Codecov Report
@@ Coverage Diff @@
## main #1677 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 169 169
Lines 14517 14523 +6
=========================================
+ Hits 14517 14523 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Summary: Pull Request resolved: pytorch#1676 The usage of `kwargs` in `optimize_acqf` and related functions has a few downsides: - It's not transparent to the user what options are supported - When kwargs get mutated by `.pop()`, this can cause subtle errors - If a user provides an unsupported option, there will be no error or warning. In this diff I change some options in `kwargs` to named arguments. I did not entirely remove `kwargs`, because they serve a few purposes, all of which are kind of iffy. I'll attack that in the next diff in the stack. These purposes are: - Some kwargs, now marked as `ic_gen_kwargs`, get passed from `optimize_acqf` to a function that generates ICs. This will only happen if the user provides a function with a different signature from the BoTorch IC generator functions. I'm not sure anyone was actually using that functionality. Tests still pass if the `kwargs` arguement in `optimize_acqf` is removed. - Users may pass incorrect keyword arguments for no good reason. This fails silently. - Ax `botorch_modular` passes the same signature to a variety of optimizers when the user does not specify otherwise. So it passes keyword arguments that don't do anything, but for a good reason. I think it would make sense to have BoTorch raise a warning and Ax silence it. Differential Revision: https://internalfb.com/D43200823 fbshipit-source-id: b66ac13dba40ccd9e0b2c4e3ef10475d9b3a8167
…h#1421) Summary: X-link: facebook/Ax#1421 Pull Request resolved: pytorch#1677 The previous diff discusses issues with kwargs in BoTorch optimizers and resolves some of them. This diff warns if keyword arguments that do nothing are passed to an optimizer, and stops Ax (botorch-modular) from doing that. Reviewed By: lena-kashtelyan Differential Revision: D43277102 fbshipit-source-id: f22b4cd1c4b6227a5210f27d1501c3667209a1d7
|
This pull request was exported from Phabricator. Differential Revision: D43277102 |
esantorella
force-pushed
the
export-D43277102
branch
from
e41ef33 to
c728104
Compare
esantorella
added a commit
to esantorella/Ax
that referenced
this pull request
Feb 14, 2023
…ok#1421) Summary: Pull Request resolved: facebook#1421 X-link: pytorch/botorch#1677 The previous diff discusses issues with kwargs in BoTorch optimizers and resolves some of them. This diff warns if keyword arguments that do nothing are passed to an optimizer, and stops Ax (botorch-modular) from doing that. Reviewed By: lena-kashtelyan Differential Revision: D43277102 fbshipit-source-id: e3a473ef47018e32b8432332033110db06b006be
facebook-github-bot
pushed a commit
to facebook/Ax
that referenced
this pull request
Feb 16, 2023
Summary: Pull Request resolved: #1421 X-link: pytorch/botorch#1677 The previous diff discusses issues with kwargs in BoTorch optimizers and resolves some of them. This diff warns if keyword arguments that do nothing are passed to an optimizer, and stops Ax (botorch-modular) from doing that. Reviewed By: lena-kashtelyan Differential Revision: D43277102 fbshipit-source-id: b31ef580223fd9d8596a9c69ac4035cbd9e8f3ef
|
This pull request has been merged in 6940b53. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Jun 26, 2024
Summary: ## Motivation This code was deprecated somewhere between 0.8.0 and 0.9.0; as we are now past 0.11.0, it can be reaped. Pull Request resolved: #2390 Test Plan: Existing units, including tutorials. ## Related PRs #1677 Reviewed By: Balandat Differential Revision: D58930631 Pulled By: esantorella fbshipit-source-id: fc6c3900baa53fa8d04299a1e398f23aa1f3aa53
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: The previous diff discusses issues with kwargs in BoTorch optimizers and resolves some of them. This diff warns if keyword arguments that do nothing are passed to an optimizer, and stops Ax (botorch-modular) from doing that.
Differential Revision: D43277102