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][docs] fix doc search issues, examples gallery & filter #31635

Merged
merged 9 commits into from
Jan 28, 2023
Merged

[train][docs] fix doc search issues, examples gallery & filter #31635

merged 9 commits into from
Jan 28, 2023

Conversation

maxpumperla
Copy link
Contributor

@maxpumperla maxpumperla commented Jan 12, 2023 •
edited
Loading

@maxpumperla
tf logo
42c663e 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
typo
8a56ee5 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
Copy link
Contributor Author

@matthewdeng can I get a review from you?!

krfricke
krfricke reviewed Jan 12, 2023
View reviewed changes
Copy link
Contributor

@krfricke krfricke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is great!

My main confusion is that the "hover" color is almost the same as the "active" color which makes it hard to see if I am activating/deactivating a filter

doc/source/_static/js/tags.js Outdated Show resolved Hide resolved
@maxpumperla
Copy link
Contributor Author

@krfricke thanks, great comments. will address both later

matthewdeng
matthewdeng approved these changes Jan 12, 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.

This is really nice! I feel like I learned some neat things here - is this in itself something we can codify as best practices?

doc/source/_toc.yml Outdated Show resolved Hide resolved
doc/source/train/architecture.rst
which is why Ray Train provides ``Backend`` implementations (e.g. ``TorchBackend``)
that can be used to run the training process using a `BackendExecutor`.

Here's a visual overview of the architecture components of Ray Train:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename this guide to be deep learning specific? The boosted tree trainers actually don't have any of these backend/executor/worker group abstractions.

Also, this diagram is severely outdated 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know... man, this particular doc is painful to read. That's why I touched it up, but it's a bit of a rabbit hole!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a TODO, but we need to commit to fixing this. I think having a general Architecture Guide is useful to have.

doc/source/train/architecture.rst Outdated Show resolved Hide resolved
doc/source/train/examples.rst Outdated Show resolved Hide resolved
amogkam
amogkam reviewed Jan 12, 2023
View reviewed changes
Copy link
Contributor

@amogkam amogkam left a comment

Choose a reason for hiding this comment

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

Thanks @maxpumperla, this looks great! Left some comments

doc/source/train/config_guide.rst Show resolved Hide resolved
doc/source/train/dl_guide.rst Show resolved Hide resolved
doc/source/_toc.yml Outdated Show resolved Hide resolved
doc/source/conf.py Show resolved Hide resolved
doc/source/train/api.rst Show resolved Hide resolved
doc/source/train/examples.rst Show resolved Hide resolved
maxpumperla and others added 4 commits January 13, 2023 09:57
@maxpumperla @amogkam
Update doc/source/_toc.yml
cd1c06f 
Co-authored-by: Amog Kamsetty <amogkam@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
rm console log
9c743a0 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
benchmark and hf logo
ff26038 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
fix css, links, ignore generated apis
c779d92 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
Copy link
Contributor Author

maxpumperla commented Jan 13, 2023 •
edited
Loading

@matthewdeng yes, I think we can codify this. I've summarised the approach here: https://docs.google.com/document/d/1cXb1FELZme9zxEYhXXxrGvtLB2xs1auHA46v-1uMYf0/edit#heading=h.8h9qzb62266g

The gallery improvements of this PR are not part of that, but having consistent, self-explaining headers (esp. for examples) is. If we want to boost certain concepts and workloads, ideally we have documents with titles corresponding to these searches.

One example: it's difficult to find good search results (on-site or search engines) on "distributed training with PyTorch/TensorFlow (on Ray)". We cover these workloads in the examples touched in this PR. But if we're literally never mentioning what we're doing anywhere, no paid doc search service can salvage that.

@maxpumperla
hover color
a8e6105 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@maxpumperla
Merge branch 'master' into mp_doc_search_train
3e22686 
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
krfricke
krfricke reviewed Jan 25, 2023
View reviewed changes
Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

Happy with the content, just two minor comments for the tags

doc/source/_static/css/custom.css Outdated Show resolved Hide resolved
doc/source/_static/js/tags.js Show resolved Hide resolved
@maxpumperla @krfricke
Update doc/source/_static/css/custom.css
289e39c 
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
Signed-off-by: Max Pumperla <max.pumperla@googlemail.com>
@richardliaw richardliaw merged commit cc6d30a into master Jan 28, 2023
@richardliaw richardliaw deleted the mp_doc_search_train branch January 28, 2023 09:54
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@maxpumperla @amogkam
@krfricke @edoakes
[train][docs] fix doc search issues, examples gallery & filter (ray-p…
898c942 
…roject#31635)

Co-authored-by: Amog Kamsetty <amogkam@users.noreply.github.com>
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krfricke krfricke krfricke left review comments

@amogkam amogkam amogkam left review comments

@richardliaw richardliaw richardliaw approved these changes

@matthewdeng matthewdeng matthewdeng approved these changes

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

@Yard1 Yard1 Awaiting requested review from Yard1

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

Successfully merging this pull request may close these issues.
5 participants
@maxpumperla @matthewdeng @richardliaw @amogkam @krfricke