DOC: split quantization.rst into smaller pieces #41321
Conversation
mattip
requested review from
apaszke,
ebetica,
goldsborough and
yf225
as code owners
facebook-github-bot
added
the
oncall: jit
|
|
||
|
|
||
| .. _interpreting-graphs: | ||
|
|
There was a problem hiding this comment.
these two labels are needed for references in jit._script.py
| tiny float The smallest positive representable number. | ||
| resolution float The approximate decimal resolution of this type, i.e., ``10**-precision``. | ||
| ========= ===== ======================================== | ||
| ========== ===== ======================================== |
There was a problem hiding this comment.
The addition of the "resolution" row broke formatting. This happened sometime in the past week, making the case stronger for not allowing warnings to pass, gh-38011
|
|
||
| /// The `Kullback-Leibler divergence`_ Loss | ||
| /// The Kullback-Leibler divergence loss measure | ||
| /// See https://pytorch.org/docs/master/nn.html#torch.nn.KLDivLoss to learn |
There was a problem hiding this comment.
The tag line of the C++ function ends up in the documentation (in the table under that link). No need to use reference formatting, since the link is in the code and the full docstring properly formats the link.
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
LGTM modulo the one broken link issue. The main quantization page looks a lot better.
I would move this conversation to gh-38011. |
|
Folks. I added Raghu and Vasiliy as reviewers for this PR. They are the content matter experts on quantization and we should get their input on the impact of this before landing this change. |
There are no significant changes in content, so I'd recommend landing this quickly to finally get CI to ensure we stay at zero doc build warnings. @raghuramank100 or @vkuzo, would you be able to look at this soon-ish? |
vkuzo
left a comment
vkuzo
left a comment
There was a problem hiding this comment.
the quantization part LGTM. Thanks for doing this, it should read much better after this change.
mruberry
requested review from
mruberry
and removed request for
apaszke,
goldsborough and
yf225
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mruberry has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Merge after gh-41334 and gh-41321 (EDIT: both are merged). Closes gh-38011 This is the last in a series of PRs to build documentation without warnings. It adds `-WT --keepgoing` to the shpinx build which will [fail the build if there are warnings](https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-W), print a [trackeback on error](https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-T) and [finish the build](https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-keep-going) even when there are warnings. It should fail now, but pass once the PRs mentioned at the top are merged. Pull Request resolved: #41335 Reviewed By: pbelevich Differential Revision: D22794425 Pulled By: mruberry fbshipit-source-id: eb2903e50759d1d4f66346ee2ceebeecfac7b094
7 participants
xref gh-38010 and gh-38011.
After this PR, there should be only two warnings:
If tensorboard and torchvision are prerequisites to building docs, they should be added to the
requirements.txt.As for breaking up quantization into smaller pieces: I split out the list of supported operations and the list of modules to separate documents. I think this makes the page flow better, makes it much "lighter" in terms of page cost, and also removes some warnings since the same class names appear in multiple sub-modules.