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.

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

Creates a new table of contents to organize content #7988

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mikegre-google
Copy link
Collaborator

I moved all content files into a directory under /docs/source so they will be picked up by the doc build process. I converted the markdown files to restructedText to clean up the TOC. I did this because all headers in a markdown file are added as a node in the TOC which results in a TOC that is way too long. I shortened some of the file names as well.

mikegre-google and others added 3 commits September 10, 2024 19:43
I moved all content files into a directory under /docs/source so they
will be picked up by the doc build process. I converted the markdown
files to restructedText to clean up the TOC. I did this because all headers in a markdown
file are added as a node in the TOC which results in a TOC that is way
too long. I shortened some of the file names as well.
Copy link
Collaborator

@will-cromar will-cromar left a comment

Choose a reason for hiding this comment

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

Thanks, Mike. I started a CI run so I can look at the github pages docs preview.

I'm still not sure about converting everything to RST -- just about everyone knows markdown, but I don't know how much of the team actually knows RST. Even if it's easy to learn, I don't want that to be a barrier or source of friction. Can you give more details about the issue you were seeing with the TOC? There may be some sphinx setting we can tweak to make MD work. IIUC anything not in index.rst would not show up as a TOC node.

CONTRIBUTING.md Outdated
@@ -1,181 +0,0 @@
# Contribute To PyTorch/XLA
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO developer-oriented documentation (at least CONTRIBUTING) should stay at the top level or in docs, since contributors will probably look for instructions on our GitHub repository.

Moving API guide is a good idea, since that's user-facing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved the content to the docs/source directory because that is where Sphinx is looking for it. None of the docs outside of the source directory were being picked up by the doc build process. I'm OK with leaving CONTRIBUTING in the docs directory. I prefer to not have all docs in a single directory because that makes it difficult to find a file you want to update. We can add a comment in CONTRIBUTING about how we structure the docs to explain how to find a file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will look into using markdown. The docs use the Pytorch Sphinx theme, perhaps there is a TOC setting that would help.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was able to get markdown files to work. I've pushed the updates to the branch I used when I created the PR.

docs/source/learn/pjrt.rst Outdated Show resolved Hide resolved
docs/source/learn/bazel.rst Outdated Show resolved Hide resolved
@will-cromar
Copy link
Collaborator

The sidebar is looking better for sure:
image

Is it possible to keep some sort of "getting started" content on the landing page, at least the pip install instructions and a basic example? This is where I land after this PR:

image

@mikegre-google
Copy link
Collaborator Author

mikegre-google commented Sep 16, 2024 via email

@@ -0,0 +1,71 @@
# Fix attention ops

Copy link
Collaborator

Choose a reason for hiding this comment

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

where is this doc coming from? It felt like a prt of the export but does not have much context and is confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This content came from xla/docs/FIX_LOWERING_FOR_CORE_ATEN_OPS.md

Since it doesn't have much content or context, I will remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can just add it back to the FIX_LOWERING_FOR_CORE_ATEN_OPS I guess

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The file has been renamed to fix_aten_ops.md. I can re-add it, but if it is confusing, without context, and without a specific purpose, I'd rather leave it out.

@@ -0,0 +1,149 @@
# Automatic Mixed Precision
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be part of perf, not contribute

Copy link
Collaborator

Choose a reason for hiding this comment

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

or rather just have a separate features directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The directory names are loosely CUJ based and are following the convention we use in the cgc documentation. I think this doc would be fine in the perf directory, so I will move it there.

@@ -1,13 +1,9 @@
Torch Export to StableHLO
--------------------------
# Torch Export to StableHLO
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be in a separate feature dir, this is not a contribute doc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact only

[Bazel in Pytorch/XLA](https://github.com/pytorch/xla/pull/7988/files#)
[Codegen migration Guide](https://github.com/pytorch/xla/pull/7988/codegen_migration.html)
[OP Lowering Guide](https://github.com/pytorch/xla/pull/7988/op_lowering.html)
[Custom Hardware Plugins](https://github.com/pytorch/xla/pull/7988/plugins.html)

belong to Contribute to Pytorch/XLA. Please move everything else in a feature dir.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A directory named "feature" doesn't fit with the rest of the directory names. They each indicate some sort of action the reader is taking: learning about PT/XLA, contributing to PT/XLA, etc. The topics currently under "contribute" are only useful for those customers that will be contributing to the PT/XLA project, so I think it makes sense to put them there. @will-cromar had suggested I place them there as well. Please explain why you feel these topics don't belong in "contribute".

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO contribute are a set of docs for the contributor, it can includes things like how to setup developer env, how to contribute a codegen/op lowering.

Export on the other hand is a feature that we want user to use. It is used to exporting a model to a stableHLO. Contributor does not need to know how export works, but we want user to see this doc and aware this feature exists. sample arguments applied to other docs. Some of the features are for the perfomrances, some features don't but they are still user facing doc, not contributor facing.

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.

3 participants