[C++ API] Document BatchNorm and update default behavior #11484
Conversation
goldsborough
requested review from
apaszke,
colesbury,
ebetica,
ezyang,
gchanan,
soumith and
zdevito
as code owners
|
Now do we want the |
|
|
|
@pytorchbot retest this please |
|
I don't have a good opinion about the momentum parameter, I might defer this to some of the vision folks? I think it might be a discussion for a future PR in any case. |
|
@prigoyal said that momentum is a universally well understood term so renaming it here probably doesn't make too much sense. |
ezyang
left a comment
ezyang
left a comment
There was a problem hiding this comment.
Not sure why you need code owner review on this one.
soumith
left a comment
soumith
left a comment
There was a problem hiding this comment.
Do not diverge the PyTorch and C++ APIs. If we have BatchNorm momentum at the python level, for better or worse we stick to it.
API divergence is a much worse experience than academic correctness.
This reverts commit 27de526152e2ee184a632d4826386f0e7f3f19d0.
goldsborough
force-pushed
the
doc-batchnorm
branch
from
27de526 to
5209943
Compare
goldsborough
dismissed
soumith’s stale review
Addressed in latest revert commit
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
* master: (165 commits) Aibench for asr decoder Explicitly set locale on docs build. (pytorch#11595) Documentation for debugging JIT Fused weightnorm for ATen (pytorch#10842) Move Type, Tensor, TensorMethods to core. Add reminder % to the jit Fix reloading modules back into python (pytorch#11552) Add trigonometry functions to docs/source/onnx.rst Add EndToEndHybridModel CUDA tests (pytorch#11544) minor formatting error log (pytorch#11528) Warn that export+import module always load onto the CPU (pytorch#11485) caffe2::StorageImpl use at::DataPtr (pytorch#11282) Sync all libnccl soversions, not just libnccl.so.1 (pytorch#11575) Document BatchNorm and update default behavior (pytorch#11484) Typo fix in randomness.rst (pytorch#11571) Move some bmm/baddbmm to ATen (pytorch#11292) Make c10d test work on CPU only build (pytorch#11567) Clean up some C++ cruftiness in the script lexer. Allow setting deletion constant Make C10d support CPU only build (pytorch#11513) ...
6 participants
This PR:
BatchNorm,statefulparameter used to befalse, but the most common usage ofBatchNormout of the wild is certainly stateful, and the default in Python is also statefulness. So we change the default to stateful.pure_forwardfunction used to use the internal running mean and variance variables instead of the ones supplied to that function call whenstatefulwas true, which certainly seems odd. When you callpure_forwardyou would certainly expect the values you pass explicitly to be used. This is now fixed.BatchNorm, finally.@ebetica @apaszke @ezyang