Skip to content

Model doc revamp - first PR #5821

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
Merged
merged 10 commits into from
Apr 19, 2022
Merged

Model doc revamp - first PR #5821

merged 10 commits into from
Apr 19, 2022

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Apr 14, 2022 •
edited
Loading

This PR introduces our new models docs, as discussed and designed in previous PRs (see below) . For now I just wrote docs for VGG and ResNet models, and generated the weights table for classification.

Rendered docs: https://output.circle-artifacts.com/output/job/aaddb6cd-567c-4552-90fa-296878ff5ab1/artifacts/0/docs/models.html

We now have:

  • a main Models page with a similar structure to what we have now (classification, detection, optical flow, etc.). This page contains links to:
    • One page per model - i.e. one page for Resnet, one for VGG, etc. Each of these pages contain links to:
    • A table summarizing all the available weights, and their accuracies. We'll do that for classification, detection, Optical flow, etc.

TODO, in future PRs:

  • Find a way to reduce the row size of the generated tables. They're too big right now.
  • Generate tables for detection, optical flow, etc.
  • Document all the remaining models. Might involve external community on this, TBD.
  • After that, write the "narrative" docs about how to use the new weights API, etc. Then go through the examples in the gallery and update them too.
  • once everything is done: decide on a way to document the recipes. Right now we just link to a script or a PR or an issue, but there might be a more suitable way to do this.

Closes #5741
Closes #5577
Closes #5575

@NicolasHug NicolasHug requested a review from datumbox April 14, 2022 14:23
NicolasHug
NicolasHug commented Apr 14, 2022
View reviewed changes
docs/Makefile
@@ -6,7 +6,7 @@ ifneq ($(EXAMPLES_PATTERN),)
endif

# You can set these variables from the command line.
SPHINXOPTS = -W -j auto $(EXAMPLES_PATTERN_OPTS)
SPHINXOPTS = -j auto $(EXAMPLES_PATTERN_OPTS)
Copy link
Member Author

Choose a reason for hiding this comment

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

The -W flag will make the docs error on warnings. It's good to have to avoid having broken references. But since I removed almost all of the models.rst file, a few of the refs (e.g. from gallery examples) are broken. To avoid noise I'm removing the flag for now.

Will definitely put it back once everything is done.

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 add a TODO or create an issue to avoid forgetting?

datumbox
datumbox reviewed Apr 19, 2022
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Thanks @NicolasHug.

Overall the proposal looks good. I've added a few comments below, let me know your thoughts.

docs/Makefile
@@ -6,7 +6,7 @@ ifneq ($(EXAMPLES_PATTERN),)
endif

# You can set these variables from the command line.
SPHINXOPTS = -W -j auto $(EXAMPLES_PATTERN_OPTS)
SPHINXOPTS = -j auto $(EXAMPLES_PATTERN_OPTS)
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 add a TODO or create an issue to avoid forgetting?

docs/source/models.rst Outdated

torchvision.models.optical_flow.raft_large
torchvision.models.optical_flow.raft_small
TODO: Something similar to classification models: list of models + table of weights
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to consider Quantized Classification models like a separate submodule (similar to detection and classification) and add documentations, tables with accuracies etc.

datumbox
datumbox approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Approving as in principle this fixes most of the major issues of the docs.

I recommend resolving #5821 (comment) prior merging to avoid leaving the live docs in a bad state.

@NicolasHug NicolasHug merged commit 111fe85 into pytorch:main Apr 19, 2022
@NicolasHug NicolasHug mentioned this pull request Apr 19, 2022
Merged
facebook-github-bot pushed a commit that referenced this pull request May 5, 2022
@YosuaMichael @facebook-github-bot
[fbsync] Model doc revamp - first PR (#5821)
91606f0 
Summary:
* First PR for model doc revamp

* Deactivating fail on warning, temporarily

* Remove commnet

* Minor changes

* Typos

* Added TODO in Makefile

* Keep old models.rst file intact, move new docs into new models_new.rst file

Reviewed By: jdsgomes, NicolasHug

Differential Revision: D36095696

fbshipit-source-id: ac5219c6b41b904b3c1197626e60788a215ede82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datumbox datumbox datumbox approved these changes

Assignees
No one assigned
Labels
cla signed enhancement module: documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NicolasHug @datumbox @facebook-github-bot