Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[tune] New persistence mode cleanup: Trainable, FunctionTrainable, Tune session #40181

Merged
merged 35 commits into from
Oct 9, 2023
Merged

[tune] New persistence mode cleanup: Trainable, FunctionTrainable, Tune session #40181

merged 35 commits into from
Oct 9, 2023

Conversation

justinvyu
Copy link
Contributor

@justinvyu justinvyu commented Oct 6, 2023 •
edited
Loading

Why are these changes needed?

This PR removes the last of the old persistence mode codepath:

  • The Trainable interface has been stripped down to just the main save, restore, train, etc. interfaces. All of the code relating to checkpoint and artifact uploading to cloud has been removed -- this is all handled by the StorageContext now.
  • The FunctionTrainable interface has been greatly simplified, and the Tune session (_StatusReporter) has been removed, since the FunctionTrainable just uses the _TrainSession now.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@justinvyu
result grid
9308a2d 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
tune.py
955c341 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
tuner.py
61957db 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
exp analysis
a4191cf 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
experiment_state
ef7c22b 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
tune controller
86e89e1 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove unused tune controller kwargs
23de124 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
experiment
1904f81 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
trial
66e8c06 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix broken remainder references
0232733 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
tuner internal
addef1e 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove some more unneeded FF checks
dc18241 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
nit
bfee05c 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
function trainable pt 1
3f1a4ab 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove function trainable util
e0f7bd0 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove remote checkpoint dir usage
44fd0c2 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix broken test util
50cc0c2 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
cleanup session
5f693d3 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove status reporter (tune session)
dc291cd 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove trainable syncing code
f94c452 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove remaining legacy path in trainable
5b8b84d 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove missing method
8002b21 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove trial unused get trial ckpts method
6e28909 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove path parsing utils
eeca53b 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix broken tune trainable session imports
c2f1ca6 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into clea…
224f3d2 
…nup_old_codepath_tune2
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into clea…
4d67b90 
…nup_old_codepath_tune2

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
check if storage exists for trainable
f4fb01a 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
add back is_session_enabled for xgboost ray
298d9f2 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove unused syncer code
a492da5 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove unused storage path parsing code
30943f3 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove save_to_obj and restore_from_obj from api refs
85a7d1b 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu justinvyu merged commit 5abc4ab into ray-project:master Oct 9, 2023
58 of 60 checks passed
@justinvyu justinvyu deleted the cleanup_old_codepath_tune2 branch October 9, 2023 00:27
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
@justinvyu
[tune] New persistence mode cleanup: Trainable, FunctionTrainable…
91c2905 
…, Tune session (ray-project#40181)

This PR removes the last of the old persistence mode codepath:
* The `Trainable` interface has been stripped down to just the main `save`, `restore`, `train`, etc. interfaces. All of the code relating to checkpoint and artifact uploading to cloud has been removed -- this is all handled by the `StorageContext` now.
* The `FunctionTrainable` interface has been greatly simplified, and the Tune session (`_StatusReporter`) has been removed, since the `FunctionTrainable` just uses the `_TrainSession` now.

---------

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Victor <vctr.y.m@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewdeng matthewdeng matthewdeng approved these changes

@sven1977 sven1977 Awaiting requested review from sven1977 sven1977 is a code owner

@gjoliver gjoliver Awaiting requested review from gjoliver

@avnishn avnishn Awaiting requested review from avnishn

@ArturNiederfahrenhorst ArturNiederfahrenhorst Awaiting requested review from ArturNiederfahrenhorst ArturNiederfahrenhorst is a code owner

@smorad smorad Awaiting requested review from smorad

@maxpumperla maxpumperla Awaiting requested review from maxpumperla maxpumperla is a code owner

@kouroshHakha kouroshHakha Awaiting requested review from kouroshHakha

@krfricke krfricke Awaiting requested review from krfricke

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@justinvyu @matthewdeng