Skip to content

[Doc][Tune][Restore] Split "Tune stopping and resuming" into two user guides - #33495

Merged
richardliaw merged 21 commits into
ray-project:masterfrom
justinvyu:doc/restore
Mar 23, 2023
Merged

[Doc][Tune][Restore] Split "Tune stopping and resuming" into two user guides#33495
richardliaw merged 21 commits into
ray-project:masterfrom
justinvyu:doc/restore

Conversation

@justinvyu

@justinvyu justinvyu commented Mar 21, 2023

Copy link
Copy Markdown
Contributor

Summary

This PR splits the "Tune Stopping and Resuming" guide into two user guides: (1) Tune Stopping, and (2) Tune Fault Tolerance. This PR also adds a FAQ post about the "iterative experimentation" workflow not being supported by Tuner.restore, but rather through new Tune experiments.

Why?

The scope of Tuner.restore functionality is unclear. Users think that it should be used for iterative experimentation where hyperparameters/configs are changed before resuming training. However, its usage should be limited to providing fault tolerance at the experiment level, without any changes to the experiment specification. [github, slack, slack, slack]

In previous discussions (Tuner API Design), we have decided against allowing modifications to things like trial stopping criteria upon resume. The idea is that an experiment should follow a single spec, and experiment restore is used for fault-tolerance to continue retrying the job if it’s interrupted. We have not communicated this decision through to outward-facing docs and usage examples (both in the open-source and product documentation), which is the reason for the continued user questions (see links above).

--> For reviewers <--

There are 3 new pieces of the docs to review:

Here's what the old guide looks like: https://docs.ray.io/en/releases-2.3.0/tune/tutorials/tune-stopping.html

Related issue number

Closes #30223
Closes #31830

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 :(

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>

@gjoliver gjoliver left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

very nice man.
so super quick questions

Comment thread doc/source/tune/doc_code/faq.py Outdated
"num_epochs": 10,
"hyperparam": tune.grid_search([4, 5, 6]),
# Use the `best_checkpoint_uri` from above.
"start_from_checkpoint": best_checkpoint_path,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is start_from_checkpoint a new config name that is introduced?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, this is just some random config name for users to seed the new run with a checkpoint. This actually only works if the checkpoint is available in s3. Otherwise, if it's a local path, some nodes will not find it.

Comment thread doc/source/tune/doc_code/fault_tolerance.py Outdated
Comment thread doc/source/tune/doc_code/fault_tolerance.py
Comment thread doc/source/tune/faq.rst

Therefore, ``Tuner.restore`` is not suitable for our desired behavior.
This style of "iterative experimentation" should be done with *new* Tune experiments
rather than restoring a single experiment over and over and modifying the experiment spec.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

but if we run new Tune experiments, how do you seed the initial model with whatever checkpint avaialble to us?
shouldn't we add an example for that too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See the first point. Without introducing some API to pass an initial checkpoint to Tuner, then accessing it through session.get_checkpoint, we need to just pass it through the config.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
…restore

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu justinvyu added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Mar 23, 2023
@richardliaw
richardliaw merged commit 2c30a6f into ray-project:master Mar 23, 2023
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
… guides (ray-project#33495)

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
… guides (ray-project#33495)

Signed-off-by: Jack He <jackhe2345@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests-ok The tagger certifies test failures are unrelated and assumes personal liability.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AIR] AIR fault tolerance needs a centralized user guide [Tune] Scope of Tuner.restore() functionality is unclear in docs

4 participants