[train] removing gpy #59289
[train] removing gpy #59289
Conversation
Signed-off-by: elliot-barn <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes gpy as a dependency and disables tests for the PB2 scheduler, which relies on gpy. The changes are correct and align with the PR's objective. My main feedback is about the long-term plan for the PB2 scheduler. The reason for skipping tests is 'for now since gpy is no longer supported', which is slightly ambiguous. If the removal of gpy support is permanent, it would be better for maintainability to remove the PB2 code and related tests entirely, rather than just skipping them. If it's a temporary measure, adding a reference to a tracking issue would provide more clarity.
python/ray/tune/tests/test_api.py
Outdated
| @pytest.mark.skip( | ||
| reason="Skipping PB2 tests for now since gpy is no longer supported" | ||
| ) |
There was a problem hiding this comment.
The reason for skipping is a bit ambiguous ('for now' vs 'no longer supported'). If gpy is permanently removed, consider removing the PB2 scheduler and its related tests altogether instead of just skipping them. This would improve maintainability by removing dead code. This would involve deleting python/ray/tune/schedulers/pb2.py, python/ray/tune/schedulers/pb2_utils.py, and removing the tests in python/ray/tune/tests/test_trial_scheduler_pbt.py and this test.
If this is a temporary measure, could you add a link to a tracking issue in the reason string for future reference?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| return {"training_iteration": time, "score": val} | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="Skipping PB2 tests for now since gpy is no longer supported") |
There was a problem hiding this comment.
Bug: Module import fails without GPy dependency
The test files have top-level imports of PB2 and UCB from pb2_utils, but pb2_utils.py has import GPy at the module level. Since gpy is being removed as a dependency, these test modules will fail to import entirely with a ModuleNotFoundError. The @pytest.mark.skip decorator only skips test execution after the module loads successfully - it cannot prevent import-time failures. This will cause all tests in test_trial_scheduler_pbt.py (including non-PB2 tests) and test_api.py to fail.
Additional Locations (1)
|
@ray-project/ray-tune @ray-project/ray-train please have a look and let us know if we have your blessings to do this. |
Signed-off-by: elliot-barn <[email protected]>
removing gpy as a dependency
depends on #59152