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

[train] Legacy interface cleanup (air.Checkpoint, LegacyExperimentAnalysis) #39289

Merged
merged 27 commits into from
Sep 8, 2023
Merged

[train] Legacy interface cleanup (air.Checkpoint, LegacyExperimentAnalysis) #39289

merged 27 commits into from
Sep 8, 2023

Conversation

justinvyu
Copy link
Contributor

@justinvyu justinvyu commented Sep 5, 2023 •
edited by matthewdeng
Loading

Why are these changes needed?

Main changes:

  1. Raise an error on air.Checkpoint construction to migrate to train.Checkpoint.
  2. Rename the NewExperimentAnalysis to just be ExperimentAnalysis. Demote the old one to LegacyExperimentAnalysis.
  3. Removes the conditional aliasing of train.Checkpoint. It always points to ray.train._checkpoint.Checkpoint now. The Checkpoint's module has also been updated to ray.train.
  4. Removes air.* usage in rllib.

Related issue number

Closes #38571

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
fix vscode cmd+click
cde270f 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove some air usage
efaa967 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove air usage in rllib
575be08 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
some fixes in rllib docs
8ab29a8 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
some other leftovers
1974d8b 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
rename to LegacyCheckpoint
04c383f 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
Revert "rename to LegacyCheckpoint"
f6ae4b5 
This reverts commit 04c383f.

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove branching alias for train.Checkpoint
fe2ea64 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
mark air.Checkpoint as deprecated
9c8f7cb 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
make checkpoint error on construction
505936d 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
mark as deprecated in docstring
5496bfe 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove air checkpoint in session
5a47837 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
new EA -> EA + EA -> legacy EA
b7f4940 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
deprecate fetch_trial_dataframes
b2e13cd 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
update module of train.Checkpoint
6749623 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
justinvyu
justinvyu commented Sep 5, 2023
View reviewed changes
python/ray/train/_internal/session.py
Comment on lines -586 to -592
# TODO(justinvyu): [code_removal]
if not isinstance(checkpoint, NewCheckpoint):
raise ValueError(
"You must pass a `ray.train.Checkpoint` "
"object to `train.report`. `ray.air.Checkpoint` is deprecated."
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We instead raise in air.Checkpoint constructor now.

@justinvyu
Revert "remove air usage in rllib"
fbfe8d8 
This reverts commit 575be08.

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix FF imports
e0289b4 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix predictor dependence on old ckpt
f639220 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove old ckpt usage in hf trainer deprecation test
24efb97 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix pytorch guide
ff1a354 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into rena…
b32dcea 
…me_air_checkpoint
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 6, 2023
justinvyu and others added 6 commits September 7, 2023 14:36
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into rena…
e8a57d5 
…me_air_checkpoint

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
fix predictor from pandas udf
2cd281f 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
remove legacy doctest
e13a342 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into rena…
ac22e04 
…me_air_checkpoint
@justinvyu
remove legacy ckpt doctest
0c1a919 
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@matthewdeng
Merge branch 'master' into rename_air_checkpoint
a68cfc5
matthewdeng
matthewdeng approved these changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@matthewdeng matthewdeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


via NFL on GIPHY

@matthewdeng matthewdeng merged commit 2913e9b into ray-project:master Sep 8, 2023
66 of 73 checks passed
@justinvyu justinvyu deleted the rename_air_checkpoint branch September 8, 2023 05:23
matthewdeng added a commit to matthewdeng/ray that referenced this pull request Sep 8, 2023
@justinvyu @matthewdeng
[train] Legacy interface cleanup (air.Checkpoint, `LegacyExperiment…
a4d283f 
…Analysis`) (ray-project#39289)

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
GeneDer pushed a commit that referenced this pull request Sep 8, 2023
@matthewdeng @krfricke
@woshiyyya @justinvyu @xwjiang2010
[train][2.7][4/n] cherry-picks for documentations, tests, examples (#…
4b6e6b2 
…39468)

* [train] Fix issues in migration of tune_cifar_torch_pbt_example (#39158)

Resolves three issues that come up when migrating the `tune_cifar_torch_pbt_example` from Ray 2.6 to Ray 2.7:

1. There is a warning message because PBT uses the `_schedule_trial_save` interface. This is added to the white list attributes so it doesn't come up anymore.
2. PBT malfunctions in Python 2.7, so instead of silently failing, we raise an error and ask users to migrate
3. When users use old `ray.air.Checkpoint` APIs on `ray.train.Checkpoint`, we should raise an actionable error message.

Signed-off-by: Kai Fricke <kai@anyscale.com>

* [tune] Make Trainable.save/restore developer APIs (#39391)

Signed-off-by: Kai Fricke <kai@anyscale.com>

* [Telemetry] Add Telemetry for Ray Train Utilities (#39363)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

* [train] update Train API references & annotations (#39294)

Signed-off-by: Matthew Deng <matt@anyscale.com>

* [2.7] Cleanup all LightningTrainer Mentions in Ray Doc (#39406)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

* [train] remove _max_cpu_fraction_per_node (#39412)

Signed-off-by: Matthew Deng <matt@anyscale.com>

* [train] Legacy interface cleanup (`air.Checkpoint`, `LegacyExperimentAnalysis`) (#39289)

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: matthewdeng <matt@anyscale.com>

* [Train][Telemetry] Limit the usage of `ray.train.torch.get_device`. (#39432)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

* [train-ci] Fix Train examples with authentication buildkite commands. (#39387)

* [train-ci] fix Train examples with authentication buildkite commands.

Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>

* [train][doc] Remove preprocessor reference in tune+train user guide (#39442)

Signed-off-by: Justin Yu <justinvyu@anyscale.com>

* [train/docs] Extend resource guide (training backend + choosing resources) (#39202)

Signed-off-by: Kai Fricke <kai@anyscale.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>

* fix docs

Signed-off-by: Matthew Deng <matt@anyscale.com>

* [Minor] Remove remaining LightningTrainer Mentions (#39441)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

---------

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Matthew Deng <matt@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
Co-authored-by: Yunxuan Xiao <yunxuanx@anyscale.com>
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com>
jimthompson5802 pushed a commit to jimthompson5802/ray that referenced this pull request Sep 12, 2023
@justinvyu @matthewdeng @jimthompson5802
[train] Legacy interface cleanup (air.Checkpoint, `LegacyExperiment…
b29cf09 
…Analysis`) (ray-project#39289)

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
Signed-off-by: Jim Thompson <jimthompson5802@gmail.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
@justinvyu @matthewdeng
[train] Legacy interface cleanup (air.Checkpoint, `LegacyExperiment…
cb4f2ad 
…Analysis`) (ray-project#39289)

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: matthewdeng <matt@anyscale.com>
Signed-off-by: Victor <vctr.y.m@example.com>
@rickyyx rickyyx mentioned this pull request Oct 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericl ericl ericl approved these changes

@matthewdeng matthewdeng matthewdeng approved these changes

@kouroshHakha kouroshHakha kouroshHakha approved these changes

@richardliaw richardliaw Awaiting requested review from richardliaw

@krfricke krfricke Awaiting requested review from krfricke

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

@amogkam amogkam Awaiting requested review from amogkam

@Yard1 Yard1 Awaiting requested review from Yard1

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

@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

Assignees

@ericl ericl

@matthewdeng matthewdeng

Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[train][GA] Alias new Checkpoint to train.Checkpoint
4 participants
@justinvyu @ericl @matthewdeng @kouroshHakha