Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confused about AveragePool op ceil_mode attribute #3314

Closed
blacklong28 opened this issue Mar 5, 2021 · 6 comments · Fixed by #5248
Closed

Confused about AveragePool op ceil_mode attribute #3314

blacklong28 opened this issue Mar 5, 2021 · 6 comments · Fixed by #5248
Labels
documentation Issues related to ONNX documentation operator Issues related to ONNX operators question Questions about ONNX

Comments

@blacklong28
Copy link

blacklong28 commented Mar 5, 2021

Ask a Question

Question

When I was study the AveragePool op, I am confused about the ceil_mode attr.
What I see at AveragePool

ceil_mode : int (default is 0)
Whether to use ceil or floor (default) to compute the output shape.

But I notice when auto_pad is set,
it only uses ceil to compute the output shape.
Is the ceil_mode attr only used when auto_pad is 'NOTSET'?

Further information

  • This is the python code I find to compute the output shape, the code also is only use ceil to compute output shape. get_output_shape
  • I notice pytorch's op AvgPool2d have ceil_mode( AvgPool2d, )I guess the onnx ceil_mode attr is only used when auto_pad is 'NOTSET', and when auto_pad is not 'NOTSET',the ceil_mode default value is 1 ,is true?

Who can help me? Thanks very much.

@blacklong28 blacklong28 added the question Questions about ONNX label Mar 5, 2021
@jcwchen
Copy link
Member

jcwchen commented Mar 5, 2021

Hi @blacklong28,
I think Operator document is correct and the test script in ONNX you mentioned get_output_shape is not updated while introducing ceil_mode. You can check the inference logic here in ONNXRunitme:
https://github.com/microsoft/onnxruntime/blob/master/onnxruntime/core/providers/cpu/nn/pool_attributes.h
By default, ceil_mode is 0 no matter what auto_pad is. Thanks.

@kraiskil
Copy link

kraiskil commented Mar 7, 2021

For reference - I had a similar question about ceil mode in MaxPool #2971
@jcwchen can you confirm that the onnx test scripts are wrong? Since they seem to be the de-facto acceptance tests for backends, at least I went with the script and not the documentation as the basis for my backend implementation.

@blacklong28
Copy link
Author

blacklong28 commented Mar 8, 2021

@kraiskil @jcwchen
Thank you for your attention and your answers.
I have read this code: https://github.com/microsoft/onnxruntime/blob/master/onnxruntime/core/providers/cpu/nn/pool_attributes.h
and I find the output shape compute when auto_pad is SAME_LOWER SAME_UPPER is equal to :
ceil_or_floor(static_cast(in_size + pad_needed - dilation * (kernel - 1) - 1) / stride + 1)
pad_needed = (int((in_size + stride - 1) / stride)-1)*stride+kernel-in_size
dilation = 1 for AveragePool,
output_shape =ceil_or_floor(static_cast<float>(in_size + (int((in_size + stride - 1) / stride)-1)*stride+kernel-in_size - dilation * (kernel - 1) - 1) / stride + 1)
= ceil_or_floor(
static_cast<float>(
( int( (in_size + stride - 1) / stride
)
-1)*stride
)/stride+1
)
= ceil_or_floor(int( (in_size + stride - 1)/stride-1)+1
=ceil_or_floor( int ( (in_size + stride - 1) / stride ) )
=floor ( (in_size + stride - 1) / stride )
= ceil((in_size + stride) / stride)
We know that int ( (in_size + stride - 1) / stride ) is a int number,
so no matter what the ceil_mode is, the output shape is int ( (in_size + stride - 1) / stride ),int() is a floor method.
And is equal to the python get_out_shape function:

  out_shape[i] = int(
      np.ceil(
          float(
              input_spatial_shape[i])
          / float(
              strides_spatial[i])))

from the https://github.com/microsoft/onnxruntime/blob/master/onnxruntime/core/providers/cpu/nn/pool_attributes.h,
When auto_pad is VALID,the output shape is:
ceil_or_floor( (in_size + pad_needed - dilation * (kernel - 1) - 1) / stride + 1)
= ceil_or_floor( (in_size + 0 - 1 * (kernel - 1) - 1) / stride + 1)
= ceil_or_floor( (in_size - kernel ) / stride + 1)
ceil_or_floor is depend on ceil_mode
But,the doc https://github.com/onnx/onnx/blob/master/docs/Operators.md#AveragePool

VALID: output_spatial_shape[i] = ceil((input_spatial_shape[i] - kernel_spatial_shape[i] + 1) / strides_spatial_shape[i])

The two places don't mesh well

  1. Does it mean that when auto_pad is equal to SAME_LOWER and SAME_UPPER, no matter what the value of ceil_mode is, the output_shape calculation is independent of ceil_mode?
  2. When auto pad is VAILD, which one we take as the standard?
  3. Can we choose OnnxRuntime as our standard?

@jcwchen
Copy link
Member

jcwchen commented Mar 10, 2021

HI @kraiskil @blacklong28,
Sorry for the late reply. We are still discussing which behavior is expected while using audo_pad and ceil_mode at the same time. IMO, the implementation in ORT makes more sense to me because ceil_mode should be not silent in any condition. Even if ceil_mode does not apply to auto_pad, then I think ONNX should at least forbid users to use them simultaneously to avoid confusion.

We will update here once @onnx/sig-operators has finalized the decision. It is definitely something needs to be clear in the document/ONNX test code. Thank you for bringing this up!

@jcwchen jcwchen added documentation Issues related to ONNX documentation operator Issues related to ONNX operators labels Mar 10, 2021
@stale
Copy link

stale bot commented Mar 28, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Mar 28, 2022
@stale stale bot closed this as completed Apr 19, 2022
@jcwchen
Copy link
Member

jcwchen commented Apr 20, 2022

Reopen it since this issue hasn't been resolved yet.

@jcwchen jcwchen reopened this Apr 20, 2022
@stale stale bot removed the stale label Apr 20, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 15, 2023
@jcwchen jcwchen reopened this May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues related to ONNX documentation operator Issues related to ONNX operators question Questions about ONNX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants