-
Notifications
You must be signed in to change notification settings - Fork 45
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor Inducing Point Selection to Use Allocator Classes for Enhanc…
…ed Flexibility (#377) (#435) Summary: # Description This PR proposes a solution to issue #378 . Although the implementation is fully functional and tested, it is presented as an overall proposal, open for discussion and potential refinement. ### Key Changes - **Refactor of `select_inducing_points` Function**: - **Previous Implementation**: Accepted `method` as a string (`"pivoted_chol"`, `"kmeans++"`, `"auto"`, `"sobol"`) and used conditional logic to select inducing points. - **New Implementation**: Now accepts an `InducingPointAllocator` instance, with `AutoAllocator` as the default if `allocator` is `None`. This approach allows users to directly pass allocator instances, aligning with the issue’s goal to enable flexible use of custom allocators like `GreedyImprovementReduction`. - **New `inducing_point_allocators.py` File**: - Introduces classes `SobolAllocator`, `KMeansAllocator`, and `AutoAllocator`, all implementing the `InducingPointAllocator` interface from Botorch. This modularizes allocator logic, moving it out of `select_inducing_points` while following the established base class structure. - **Modifications to Models and Example Files**: - Updated `gp_classification.py`, `monotonic_projection_gp.py`, `monotonic_rejection_gp.py`, `semi_p.py`, and `example_problems.py` to handle allocator class instances rather than string-based methods, improving overall consistency. - Added imports for the new allocator classes in `__init__.py` for cross-codebase accessibility. - **Updated Tests**: - Adjusted tests in `test_semi_p.py`, `test_utils.py`, and `test_config.py` to work with allocator classes instead of the previous string-based structure. ### Additional Notes This PR preserves most of the existing logic in `select_inducing_points` to keep changes minimal. I know further work is needed to confirm compatibility with additional Botorch allocators and to support advanced configurations using `from_config` for custom allocator setups. I’d love to hear your feedback on the overall approach before moving forward with these additional refinements. Pull Request resolved: #435 Reviewed By: crasanders Differential Revision: D65451912 Pulled By: JasonKChow fbshipit-source-id: e0529e545e428ad94ef965cc9d642577cbeb2777
- Loading branch information
1 parent
d9ee675
commit 32af5c5
Showing
13 changed files
with
1,427 additions
and
103 deletions.
There are no files selected for viewing
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
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
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
Oops, something went wrong.