CoinCheung commented on a change in pull request #10000: [MXNET-80] Fix average
pooling kernel size assignment error
URL: https://github.com/apache/incubator-mxnet/pull/10000#discussion_r173990460
##########
File path: src/operator/nn/pooling-inl.h
##########
@@ -56,11 +56,11 @@ struct PoolingParam : public dmlc::Parameter<PoolingParam>
{
DMLC_DECLARE_FIELD(cudnn_off).set_default(false)
.describe("Turn off cudnn pooling and use MXNet pooling operator. ");
- DMLC_DECLARE_FIELD(kernel)
+ DMLC_DECLARE_FIELD(kernel).set_default(TShape()) // add default value here
.enforce_nonzero()
.describe("Pooling kernel size: (y, x) or (d, y, x)");
- DMLC_DECLARE_FIELD(pool_type)
+ DMLC_DECLARE_FIELD(pool_type).set_default(pool_enum::kMaxPooling) // add
default pooling method
.add_enum("max", pool_enum::kMaxPooling)
.add_enum("avg", pool_enum::kAvgPooling)
.add_enum("sum", pool_enum::kSumPooling)
Review comment:
But I see in the original version that the order is: global_pool(false),
cudnn_off(false), kernel(no default value), pool_type(no default value), ....
In the original version, "kernel" and "pool_type" do not have default value,
but they go after "global_pool" and "cudnn_off" which are with default values.
https://github.com/apache/incubator-mxnet/blob/94f68fc8fd21611b7f5c148cb0e5d134efe58f87/src/operator/nn/pooling-inl.h#L59
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
