Skip to content

[Train][Doc] Improve AIR Lightning APIs and docstrings - #33895

Merged
gjoliver merged 10 commits into
ray-project:masterfrom
woshiyyya:train/improve-lightning-api-and-doc
Mar 31, 2023
Merged

[Train][Doc] Improve AIR Lightning APIs and docstrings#33895
gjoliver merged 10 commits into
ray-project:masterfrom
woshiyyya:train/improve-lightning-api-and-doc

Conversation

@woshiyyya

@woshiyyya woshiyyya commented Mar 29, 2023

Copy link
Copy Markdown
Member

Why are these changes needed?

During the LightningTrainer bug-bash session, we collected a lot of constructive feedback to improve the user experience. This PR aims to address these feedbacks:

LightningTrainer:

  • Updated docstring: remove "All other ModelCheckpoint callbacks will be ignored."
  • Allow users to provide their customized ProgressBar callback.

LightningCheckpoint:

  • Added examples for .from_directory() and .get_model() usage in the docstring.
  • Changed load_from_checkpoint_kwargs argument type from dict to **kwargs.
  • Added an error message in checkpoint.from_path(): if the provided path is a directory, then instruct them to provide a file path, or to use from_directory() instead.

LightningConfigBuilder:

  • Add docstrings in checkpointing(), fit_params(), module(), and trainer()
  • Raise a warning if the user provides "model" in .fit_params().
  • Tell people to still provide an AIR CheckpointConfig even if they have already set it in the LightningConfigBuilder.checkpointing().

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

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
woshiyyya marked this pull request as ready for review March 29, 2023 23:35
@woshiyyya woshiyyya changed the title [Train][Doc] Improve LightningTrainer API and docstrings [Train][Doc] Improve AIR Lightning APIs and docstrings Mar 29, 2023

@krfricke krfricke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me so far!

Comment thread python/ray/train/lightning/lightning_trainer.py Outdated
Comment thread python/ray/train/lightning/lightning_trainer.py Outdated
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Comment thread python/ray/train/lightning/lightning_checkpoint.py
Comment thread python/ray/train/lightning/lightning_checkpoint.py Outdated
Comment thread python/ray/train/lightning/lightning_checkpoint.py
Comment thread python/ray/train/lightning/lightning_predictor.py
Comment thread python/ray/train/lightning/lightning_predictor.py
Comment thread python/ray/train/lightning/lightning_trainer.py
Comment thread python/ray/train/lightning/lightning_trainer.py Outdated
Comment thread python/ray/train/lightning/lightning_trainer.py
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

@zhe-thoughts zhe-thoughts left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved for merging to master. Low risk docs / examples change, in a new component

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@gjoliver
gjoliver merged commit 643623c into ray-project:master Mar 31, 2023
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
…3895)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
…3895)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants