Skip to content

[AIR][Train][Doc] Restructure API reference - #32360

Merged
krfricke merged 21 commits into
ray-project:masterfrom
justinvyu:train/api_restructure
Feb 14, 2023
Merged

[AIR][Train][Doc] Restructure API reference#32360
krfricke merged 21 commits into
ray-project:masterfrom
justinvyu:train/api_restructure

Conversation

@justinvyu

@justinvyu justinvyu commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

Why are these changes needed?

This PR splits up long API refs in AIR and Train into individual pages, one dedicated to each method/class.

This PR is a followup to #31204 and #32311, which made the same changes for Ray Data/Tune docs.

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

@maxpumperla maxpumperla 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.

great stuff!

@justinvyu justinvyu added release-blocker P0 Issue that blocks the release P0 Issues that should be fixed in short order docs An issue or change related to documentation labels Feb 9, 2023

@c21 c21 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 by spot-checking several doc page. cc @matthewdeng for approval.

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

@matthewdeng matthewdeng 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.

Very nice!

Comment on lines +9 to +10
.. toctree::
:maxdepth: 2

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.

Love this structure so much!

Comment thread doc/source/ray-air/api/api.rst Outdated
Dataset Ingest (Ray Data)<dataset-ingest.rst>
Trainers (Ray Train)<../../train/api/api>
Tuner (Ray Tune)<../../tune/api_docs/execution>
Results (Ray Train + Ray Tune)<../../tune/api_docs/result_grid>

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.

This doesn't seem to be rendering properly on the base page or table of contents. I think it might be that the result_grid page is missing a title header or something?

image

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.

This will be fixed by merging in Tune PR.

Comment thread doc/source/train/api/api.rst Outdated
Comment on lines +75 to +101
Ray Train Built-in Predictors
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. autosummary::
:toctree: doc/

~xgboost.XGBoostPredictor
~lightgbm.LightGBMPredictor
~tensorflow.TensorflowPredictor
~torch.TorchPredictor
~huggingface.HuggingFacePredictor
~sklearn.SklearnPredictor
~rl.RLPredictor

Ray Train Framework-specific Checkpoints
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. autosummary::
:toctree: doc/

~xgboost.XGBoostCheckpoint
~lightgbm.LightGBMCheckpoint
~tensorflow.TensorflowCheckpoint
~torch.TorchCheckpoint
~huggingface.HuggingFaceCheckpoint
~sklearn.SklearnCheckpoint
~rl.RLCheckpoint

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.

Personally feel like we should move these to the predictor/checkpoint pages even though they're in the train package...

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.

I think checkpoints should stay in train, since they are still needed by users of Train only (w/o prediction). I can include another reference in the air checkpoint page for framework specific ckpts, but without toctree.

Predictors I agree should be moved out.

@@ -0,0 +1,18 @@
.. _air-serve-integration:

Model Serving in AIR

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.

Add ray.serve.air_integrations.PredictorWrapper?

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.

PredictorWrapper should be on this page already.

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.

Ahhh woops the rendering looked a bit strange because of the alias

image

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.

My mistake!

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.

Yeah, to be fair this page is kinda a mess. The see also tooltip + autosummary + autofunction is a bit much. But I had to do switch predictor deployment to an autofunction because it's an empty class that was erroring if I put it in autosummary.

Comment thread doc/source/ray-air/api/api.rst Outdated
AIR Checkpoint (All Libraries)<checkpoint.rst>
Predictors (Ray Data + Ray Train)<predictor.rst>
Model Serving in AIR (Ray Serve)<serve.rst>
External Library Integrations<../../tune/api_docs/integration>

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.

Just double checking that we're intentionally keeping these unchanged?

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.

The external libraries one will be updated with the Tune PR. The other ones are updated.

…n/api_restructure

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

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

@c21 c21 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.

cc @richardliaw - can you merge this?

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

Copy link
Copy Markdown
Contributor Author

@richardliaw Let's wait to merge after tests pass.

@richardliaw

richardliaw commented Feb 13, 2023 via email

Copy link
Copy Markdown
Contributor

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>
@krfricke
krfricke merged commit d87d86f into ray-project:master Feb 14, 2023
justinvyu added a commit to justinvyu/ray that referenced this pull request Feb 14, 2023
This PR splits up long API refs in AIR and Train into individual pages, one dedicated to each method/class.

This PR is a followup to ray-project#31204 and ray-project#32311, which made the same changes for Ray Data/Tune docs.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
justinvyu deleted the train/api_restructure branch February 16, 2023 16:53
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
This PR splits up long API refs in AIR and Train into individual pages, one dedicated to each method/class.

This PR is a followup to ray-project#31204 and ray-project#32311, which made the same changes for Ray Data/Tune docs.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
This PR splits up long API refs in AIR and Train into individual pages, one dedicated to each method/class.

This PR is a followup to ray-project#31204 and ray-project#32311, which made the same changes for Ray Data/Tune docs.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: elliottower <elliot@elliottower.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs An issue or change related to documentation P0 Issues that should be fixed in short order release-blocker P0 Issue that blocks the release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants