-
Notifications
You must be signed in to change notification settings - Fork 76
Symmetry and Data Augmentation #626
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
base: dev/symmetry
Are you sure you want to change the base?
Changes from all commits
1ae9bfd
764caca
99e1e60
221234b
e9dff9c
d1c88db
68b46a9
241a9d3
d82f8de
3d08a20
49a046e
bfb9fe9
7b9e134
fb21269
d058531
57d03e9
e8a97e8
aa783fe
36c4779
c853cac
9f076b6
be1c293
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,22 @@ def _get_acquisition_function(self, objective: Objective) -> AcquisitionFunction | |
| return qLogNEHVI() if objective.is_multi_output else qLogEI() | ||
| return self.acquisition_function | ||
|
|
||
| def _get_surrogate_for_augmentation(self) -> Surrogate | None: | ||
| """Get the Surrogate instance for augmentation/validation, if available.""" | ||
| from baybe.surrogates.composite import CompositeSurrogate, _ReplicationMapping | ||
|
|
||
| model = self._surrogate_model | ||
| if isinstance(model, Surrogate): | ||
| return model | ||
| if isinstance(model, CompositeSurrogate): | ||
| # All inner surrogates are copies of the same template | ||
| surrogates = model.surrogates | ||
| if isinstance(surrogates, _ReplicationMapping): | ||
| template = surrogates.template | ||
| if isinstance(template, Surrogate): | ||
| return template | ||
| return None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember our logic correctly, then we allow users to implement whatever surrogates they want as long as they implement the Three questions:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm in the case you describe there is also no moment when the user ever assingned any |
||
|
|
||
| def get_surrogate( | ||
| self, | ||
| searchspace: SearchSpace, | ||
|
|
@@ -114,6 +130,13 @@ def _setup_botorch_acqf( | |
| f"{len(objective.targets)}-target multi-output context." | ||
| ) | ||
|
|
||
| # Perform data augmentation if configured | ||
| surrogate_for_augmentation = self._get_surrogate_for_augmentation() | ||
| if surrogate_for_augmentation is not None: | ||
|
AVHopp marked this conversation as resolved.
|
||
| measurements = surrogate_for_augmentation.augment_measurements( | ||
| measurements, searchspace.parameters | ||
| ) | ||
|
|
||
| surrogate = self.get_surrogate(searchspace, objective, measurements) | ||
| self._botorch_acqf = acqf.to_botorch( | ||
| surrogate, | ||
|
|
@@ -156,6 +179,13 @@ def recommend( | |
|
|
||
| validate_object_names(searchspace.parameters + objective.targets) | ||
|
|
||
| # Validate compatibility of surrogate symmetries with searchspace | ||
| surrogate_for_validation = self._get_surrogate_for_augmentation() | ||
| if surrogate_for_validation is not None: | ||
| for s in surrogate_for_validation.symmetries: | ||
| s.validate_searchspace_context(searchspace) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important: Validation so far is only part of the recommend call here in the recommenders. Validation has not been included in the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AdrianSosic I see now that the 2nd point could be solved with the In the absence of that its not realy possible to turn it into an upfront validation, so I would probably not change the validation for this moment unless you have a smarter idea
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for being pragmatic and not trying to come up with something potentially convoluted right now. Even if we find a better way for the validation later, including it is just a plain improvement without negative consequences to users, so we can add it later without problems. |
||
|
|
||
| # Experimental input validation | ||
| if (measurements is None) or measurements.empty: | ||
| raise NotImplementedError( | ||
| f"Recommenders of type '{BayesianRecommender.__name__}' do not support " | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.