-
Notifications
You must be signed in to change notification settings - Fork 358
Add multi-model acquisition to MultiAcquisition #4110
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
Closed
Closed
Conversation
This file contains hidden or 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
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D78560935 |
34c94ba to
126403b
Compare
bletham
added a commit
to bletham/Ax
that referenced
this pull request
Aug 6, 2025
Summary: Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate. This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen. The diff does a refactor of `Acquisition.__init__` to move some of the logic into other methods that are called during `__init__`, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each. The `gen_metadata` entry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway. Differential Revision: D78560935
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D78560935 |
bletham
added a commit
to bletham/Ax
that referenced
this pull request
Aug 6, 2025
Summary: Pull Request resolved: facebook#4110 Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate. This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen. The diff does a refactor of `Acquisition.__init__` to move some of the logic into other methods that are called during `__init__`, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each. The `gen_metadata` entry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway. Differential Revision: D78560935
2cb9f20 to
89dd094
Compare
bletham
added a commit
to bletham/Ax
that referenced
this pull request
Aug 7, 2025
Summary: Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate. This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen. The diff does a refactor of `Acquisition.__init__` to move some of the logic into other methods that are called during `__init__`, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each. The `gen_metadata` entry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway. Differential Revision: D78560935
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D78560935 |
bletham
added a commit
to bletham/Ax
that referenced
this pull request
Aug 7, 2025
Summary: Pull Request resolved: facebook#4110 Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate. This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen. The diff does a refactor of `Acquisition.__init__` to move some of the logic into other methods that are called during `__init__`, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each. The `gen_metadata` entry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway. Differential Revision: D78560935
89dd094 to
824ef56
Compare
Summary: ModelConfig has a name field that is optional. MBM Surrogate needs an identifier for model; currently uses name if present, or just drops the model if not when storing eval results. MBM Generator needs an identifier for model and uses name if present, or str(model_config) if not. This cleans things up a bit by providing an identifier for ModelConfig that will always exist and so can be used in the place of name and clean up logic in the other parts of MBM. The identifier follows what was done in MBM Generator for tracking gen metadata: it is name if name exists, otherwise a string dump of the model config. Reviewed By: sdaulton Differential Revision: D78560884
Summary: Adds a "models_for_gen(n)" method to Surrogate that will constructs a list of n models from which to generate n candidates. The list of models is constructed by sampling from the given ModelConfigs according to their model selection eval criterion (taken as a measure of model quality). Differential Revision: D77252555
Summary: Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate. This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen. The diff does a refactor of `Acquisition.__init__` to move some of the logic into other methods that are called during `__init__`, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each. The `gen_metadata` entry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway. Differential Revision: D78560935
824ef56 to
5490dd2
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D78560935 |
Contributor
|
This pull request has been merged in 55d9a96. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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:
Augments MultiAcquisition to additionally handle the case where we want to generate from multiple models with the same botorch acquisition class. The collection of models from which to generate is created by Surrogate.
This requires making the MBM Acquisition aware of how many points are intended to be generated with this particular Acquisition. Note that we do construct a new Acquisition in every call to gen.
The diff does a refactor of
Acquisition.__init__to move some of the logic into other methods that are called during__init__, and then can be re-called when we want to use the same Acquisition object to instantiate multiple botorch acquisition functions with different models each.The
gen_metadataentry that stores which model was used for the generation is now potentially a list of models, so I renamed the entry, both to be something a little more clear and also because there is a breaking change in the data type if one were trying to do some type of meta analysis anyway.Differential Revision: D78560935