Make mean function work across multiple dimensions.
#14252
Conversation
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@umanwizard has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
umanwizard
force-pushed
the
mean_multi_dim
branch
from
05ebfc9 to
27eb37d
Compare
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@umanwizard 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.
@umanwizard has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
apaszke
left a comment
apaszke
left a comment
There was a problem hiding this comment.
fbgemm submodule has been accidentally updated
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@umanwizard has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
colesbury
left a comment
colesbury
left a comment
There was a problem hiding this comment.
LGTM, although I'm not familiar with the torch/csrc/jit/passes/shape_analysis.cpp code.
| // Additionally: | ||
| // - First input should be the only tensor input | ||
| // - has a bool keepdim argument | ||
| static const register_formula_for multidim_reduce_ops { |
There was a problem hiding this comment.
@zou3519 are you familiar with this file? Can you take a look at this change?
apaszke
left a comment
apaszke
left a comment
There was a problem hiding this comment.
@colesbury shape prop lgtm
Summary: Multi-dimensional `sum` is already implemented, and it's trivial to implement `mean` in terms of `sum`, so just do it. Bonus: Fix incomplete language in the `torch.sum` documentation which doesn't take into account multiple dimensions when describing `unsqueeze` (at the same time as introducing similar language in `torch.mean`). Pull Request resolved: pytorch/pytorch#14252 Differential Revision: D13161157 Pulled By: umanwizard fbshipit-source-id: c45da692ba83c0ec80815200c5543302128da75c
5 participants
Multi-dimensional
sumis already implemented, and it's trivial to implementmeanin terms ofsum, so just do it.Bonus: Fix incomplete language in the
torch.sumdocumentation which doesn't take into account multiple dimensions when describingunsqueeze(at the same time as introducing similar language intorch.mean).