Use regular inheritance instead of dispatcher to special-case PairwiseGP logic #2176
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
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D53011931 |
esantorella
added a commit
to esantorella/botorch
that referenced
this pull request
Jan 23, 2024
…eGP logic (pytorch#2176) Summary: * Replace dispatcher with inheritance: `Model.construct_inputs` calls a dispatcher, `parse_training_data`, that dispatches to one function if the Model is a PairwiseGP and the dataset is a RankingDataset and another for the general case of a Model and SupervisedDataset. This can be achieved with inheritance, putting the general case in Model and special case for PairwiseGP in PairwiseGP.construct_inputs. This allows the `parse_training_data` dispatcher to be removed. * Error for using an input that is not a `RankingDataset` to `PairwiseGP.construct_inputs`: Previously, if `PairwiseGP.construct_inputs` was called with a dataset that was not a `RankingDataset`, it would fall back to the general behavior for `Model.construct_inputs`, producing inputs that are not valid for `PairwiseGP`. This now produces an error. Reviewed By: saitcakmak Differential Revision: D53011931
esantorella
force-pushed
the
export-D53011931
branch
from
e168767 to
0af41b0
Compare
|
This pull request was exported from Phabricator. Differential Revision: D53011931 |
…eGP logic (pytorch#2176) Summary: * Replace dispatcher with inheritance: `Model.construct_inputs` calls a dispatcher, `parse_training_data`, that dispatches to one function if the Model is a PairwiseGP and the dataset is a RankingDataset and another for the general case of a Model and SupervisedDataset. This can be achieved with inheritance, putting the general case in Model and special case for PairwiseGP in PairwiseGP.construct_inputs. This allows the `parse_training_data` dispatcher to be removed. * Error for using an input that is not a `RankingDataset` to `PairwiseGP.construct_inputs`: Previously, if `PairwiseGP.construct_inputs` was called with a dataset that was not a `RankingDataset`, it would fall back to the general behavior for `Model.construct_inputs`, producing inputs that are not valid for `PairwiseGP`. This now produces an error. Reviewed By: saitcakmak Differential Revision: D53011931
esantorella
force-pushed
the
export-D53011931
branch
from
0af41b0 to
e927598
Compare
|
This pull request was exported from Phabricator. Differential Revision: D53011931 |
|
This pull request has been merged in 6a66c79. |
stefanpricopie
pushed a commit
to stefanpricopie/botorch
that referenced
this pull request
Feb 27, 2024
…eGP logic (pytorch#2176) Summary: Pull Request resolved: pytorch#2176 * Replace dispatcher with inheritance: `Model.construct_inputs` calls a dispatcher, `parse_training_data`, that dispatches to one function if the Model is a PairwiseGP and the dataset is a RankingDataset and another for the general case of a Model and SupervisedDataset. This can be achieved with inheritance, putting the general case in Model and special case for PairwiseGP in PairwiseGP.construct_inputs. This allows the `parse_training_data` dispatcher to be removed. * Error for using an input that is not a `RankingDataset` to `PairwiseGP.construct_inputs`: Previously, if `PairwiseGP.construct_inputs` was called with a dataset that was not a `RankingDataset`, it would fall back to the general behavior for `Model.construct_inputs`, producing inputs that are not valid for `PairwiseGP`. This now produces an error. Reviewed By: saitcakmak Differential Revision: D53011931 fbshipit-source-id: d697df3fffeaa5c9213a52a1dd39afd3b7a0ee25
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:
Model.construct_inputscalls a dispatcher,parse_training_data, that dispatches to one function if the Model is a PairwiseGP and the dataset is a RankingDataset and another for the general case of a Model and SupervisedDataset. This can be achieved with inheritance, putting the general case in Model and special case for PairwiseGP in PairwiseGP.construct_inputs. This allows theparse_training_datadispatcher to be removed.RankingDatasettoPairwiseGP.construct_inputs: Previously, ifPairwiseGP.construct_inputswas called with a dataset that was not aRankingDataset, it would fall back to the general behavior forModel.construct_inputs, producing inputs that are not valid forPairwiseGP. This now produces an error.Reviewed By: saitcakmak
Differential Revision: D53011931