Delete select_inducing_point utility function #480
Closed
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: D67068021 |
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
e534f86 to
b7f22a5
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
force-pushed
the
export-D67068021
branch
from
b7f22a5 to
70f29a1
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
force-pushed
the
export-D67068021
branch
from
70f29a1 to
41ed1c7
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
41ed1c7 to
0106707
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
0106707 to
e606e74
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
e606e74 to
7b5ec92
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 12, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
7b5ec92 to
e4d0d2e
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 13, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
e4d0d2e to
837a073
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
837a073 to
28ec942
Compare
Summary: The test_finish_criteria test uses the MonotonicRejectionGP alongside many mocks. This seems really brittle meaning that it fails when very subtle things changes in the model (and is seemingly impossible to debug and maintain). The min_post_range finish condition actually cares about the values of the model so we move this out of that test into its own acutally using a model we fit (GPClassificationModel) so that we can test it easier. Differential Revision: D67066880
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
2f70fd1 to
d14ec59
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
d14ec59 to
3f74d08
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
force-pushed
the
export-D67068021
branch
from
3f74d08 to
91d1e47
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
91d1e47 to
890d6f6
Compare
JasonKChow
added a commit
to JasonKChow/aepsych
that referenced
this pull request
Dec 14, 2024
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Differential Revision: D67068021
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
force-pushed
the
export-D67068021
branch
from
890d6f6 to
7093d02
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
JasonKChow
force-pushed
the
export-D67068021
branch
from
7093d02 to
032f210
Compare
…ncy (facebookresearch#475) Summary: Pull Request resolved: facebookresearch#475 #### PR Description: This PR builds upon the changes introduced in facebookresearch#467 by fully removing the usage of lower and upper bounds within all models. ## Key updates include: - **Removal of Bounds and Dimension from Models:** All `lb`, `ub`, and `dim` parameters have been fully removed from all models. The responsibility for computing inducing points is now entirely managed by allocators, ensuring a cleaner and more modular design. - **Dependency Shift to Allocators:** All model dependencies on dimensions (e.g., for computing the `covar_module`) are now managed through the `dim` property of the allocator object. This centralizes the handling of bounds and dimensions in the allocator system. - **Updated Allocators Functionality:** Allocators like `AutoAllocator` and `KMeansAllocator` now optionally take bounds. Bounds are essential in scenarios where no input data is provided, as they ensure fallback to the `DummyAllocator` (e.g., during initialization from configuration). - **Tests:** - All tests have been updated to make use of the allocator object as a requirement. - Most tests pass bounds to the allocator, while others initialize bounds via configuration. - Tests now fully align with the updated model-allocator interaction and function as expected. Let me know if any changes are needed! :) Pull Request resolved: facebookresearch#468 Differential Revision: D66968212
Summary: Move the inducing point allocator classes to its own directory under models and split each allocator out to their own files. Modified imports to match the refactor and cleaned up imports around them. Differential Revision: D67044580
…facebookresearch#478) Summary: Pull Request resolved: facebookresearch#478 Inducing point allocator classes had extra methods and attributes that were not necessary or misnamed. These have been cleaned up in the BaseAllocator class and all of its children are similarly updated. The dummy allocator is no longer needed, instead, the allocators can just make dummy points based on this dimensionality. This sets the last_allocator_used attribute to be None to signify no actual allocator was used in creating the last set of points. This also requires all allocators to know its dimensionality at least as it is used by the dummy allocator when there's no inputs. All models that use an allocator now initializes the allocators outside as there's no trivial default for allocators anymore. Differential Revision: D67059839
Summary: The init of GPClassification was saving exta atttributes unnecessarily as well as getting largely disorganized in the refactors. We reorganize GPClassificationModel's init and its children. Differential Revision: D67104228
Summary: Pull Request resolved: facebookresearch#480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Reviewed By: crasanders Differential Revision: D67068021
JasonKChow
force-pushed
the
export-D67068021
branch
from
032f210 to
dafb176
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67068021 |
|
This pull request has been merged in 1b38e0d. |
JasonKChow
added a commit
that referenced
this pull request
Dec 18, 2024
Summary: Pull Request resolved: #480 Unnecessary utility function replaced by just calling the Allocator class method. This diff removes using strings to select allocators. Reviewed By: crasanders Differential Revision: D67068021 fbshipit-source-id: e0613e36d8076cbfebd75a9643c8ee3794f76270
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:
Unnecessary utility function replaced by just calling the Allocator class method.
This diff removes using strings to select allocators.
Differential Revision: D67068021