conv2d / avg_pool2d implmentation for LongTensor #20105
Conversation
pytorchbot
added
module: cpu
ifedan
force-pushed
the
export-D15192353
branch
2 times, most recently
from
5c94fca to
1335a39
Compare
Summary: - enable generation of conv2d for LongTensor - enable generation of avg_pool2d for LongTensor Differential Revision: D15192353 fbshipit-source-id: fb293ca7fd647a20b1f8ded11c70b59af93937d4
gchanan
left a comment
gchanan
left a comment
There was a problem hiding this comment.
testing? It also seems like you don't support a larger number of cases, e.g. depthwise, transposed, dilated convolutions.
Maybe similar on the AvgPool / SumPool too, but I'm not sure.
There was a problem hiding this comment.
instead of doing this, can you just include Long manually where you need it?
i.e.
#include "whatever"
#include "THGenerateFloatTypes.h"
#include "whatever
#include "THGenerateLongType.h"
aten/src/THC/THCBlas.cu
Outdated
There was a problem hiding this comment.
we shouldn't define this if it's not supported (and won't be in the near future). Just throw an error in the implementation.
| scalar_check: | ||
| output: 'false' | ||
| grad_input: 'false' | ||
| scalar_types: ['Float', 'Double', 'Half', 'Long'] |
There was a problem hiding this comment.
can we support cpu_scalar_types and cuda_scalar_types since we don't actually support conv2d for longs on CUDA anyway?
| bool ceil_mode, | ||
| bool count_include_pad); | ||
|
|
||
| THC_API void THNN_(SpatialAveragePooling_updateGradInput)( |
There was a problem hiding this comment.
I don't think these backward definitions (updateGradInput, accGradParameters) make sense for integer types.
| int padW, int padH, | ||
| accreal scale); | ||
|
|
||
| TH_API void THNN_(SpatialAveragePooling_updateOutput)( |
There was a problem hiding this comment.
I don't think this makes sense to implement for long because of the integer division (i.e. we don't support long on mean). Doesn't it make sense to implement SumPool instead?
|
There is another PR: #20370 that will track this FR. This one will be closed |
3 participants
Summary:
Differential Revision: D15192353