Remove normalize_inputs and replace with parameter transform #431
+1,523
−173
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
Summary: Pull Request resolved: facebookresearch#422 Sobol generators would return the incorrect shape when handling multi stimuli generation. This would not cause problem because of the ask converted inadvertantly avoided the problem. Fixed and clarified the docstring what should happen. Sort of a bandaid fix, tensor shapes may need to be unified more carefully when n-choice models are implemented. Differential Revision: D65239074
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D65069497 |
…esearch#401) Summary: Pull Request resolved: facebookresearch#401 Parameter transforms will be handled by wrapping generator and model objects. The wrappers surfaces the base object API completely and even appears to be the wrapped object upon type inspection. Methods that requires the transformations are overridden by the wrapper to apply the required (un)transforms. The wrappers expects transforms from BoTorch and new transforms should follow BoTorch's InputTransforms. As a baseline a log10 transform is implemented. Differential Revision: D64129439
JasonKChow
force-pushed
the
export-D65069497
branch
from
35b54b3
to
156ccf0
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Nov 1, 2024
…kresearch#431) Summary: Pull Request resolved: facebookresearch#431 `normalize_inputs` (the one that min-max scales paraemters) is confusingly named (there's another `normalize_inputs` that concatenates data and ensures they're all the right types) and is a hard-coded transformation that is applied to all parameters. This means that there's no way to turn the behavior off selectively nor is it obvious that it is happening. This diff removes the normalize_inputs method and replaces it with an parameter transform that will also allow selective application of the transform via an index. Differential Revision: D65069497
|
This pull request was exported from Phabricator. Differential Revision: D65069497 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Nov 1, 2024
…kresearch#431) Summary: Pull Request resolved: facebookresearch#431 `normalize_inputs` (the one that min-max scales paraemters) is confusingly named (there's another `normalize_inputs` that concatenates data and ensures they're all the right types) and is a hard-coded transformation that is applied to all parameters. This means that there's no way to turn the behavior off selectively nor is it obvious that it is happening. This diff removes the normalize_inputs method and replaces it with an parameter transform that will also allow selective application of the transform via an index. Differential Revision: D65069497
JasonKChow
force-pushed
the
export-D65069497
branch
from
156ccf0
to
61e1535
Compare
|
This pull request was exported from Phabricator. Differential Revision: D65069497 |
Summary: Instead of creating duplicate transforms whenever we need one, we create a single transform from the config and initialize the wrapped model and wrapped generators with that one transform. This passes the same transform object around and allows the transformations to learn parameters and still be synced up across wrapped objects. Differential Revision: D65155103
|
This pull request was exported from Phabricator. Differential Revision: D65069497 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Nov 1, 2024
…kresearch#431) Summary: Pull Request resolved: facebookresearch#431 `normalize_inputs` (the one that min-max scales paraemters) is confusingly named (there's another `normalize_inputs` that concatenates data and ensures they're all the right types) and is a hard-coded transformation that is applied to all parameters. This means that there's no way to turn the behavior off selectively nor is it obvious that it is happening. This diff removes the normalize_inputs method and replaces it with an parameter transform that will also allow selective application of the transform via an index. Differential Revision: D65069497
JasonKChow
force-pushed
the
export-D65069497
branch
from
61e1535
to
cd971a0
Compare
…kresearch#431) Summary: Pull Request resolved: facebookresearch#431 `normalize_inputs` (the one that min-max scales paraemters) is confusingly named (there's another `normalize_inputs` that concatenates data and ensures they're all the right types) and is a hard-coded transformation that is applied to all parameters. This means that there's no way to turn the behavior off selectively nor is it obvious that it is happening. This diff removes the normalize_inputs method and replaces it with an parameter transform that will also allow selective application of the transform via an index. Differential Revision: D65069497
JasonKChow
force-pushed
the
export-D65069497
branch
from
cd971a0
to
1a07ce2
Compare
|
This pull request was exported from Phabricator. Differential Revision: D65069497 |
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:
normalize_inputs(the one that min-max scales paraemters) is confusingly named (there's anothernormalize_inputsthat concatenates data and ensures they're all the right types) and is a hard-coded transformation that is applied to all parameters. This means that there's no way to turn the behavior off selectively nor is it obvious that it is happening.This diff removes the normalize_inputs method and replaces it with an parameter transform that will also allow selective application of the transform via an index.
Differential Revision: D65069497