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

Problems with ResNet18 #87

Closed
tjingrant opened this issue Aug 2, 2018 · 7 comments
Closed

Problems with ResNet18 #87

tjingrant opened this issue Aug 2, 2018 · 7 comments

Comments

@tjingrant
Copy link

Hello, resnet18 referenced in this link: https://s3.amazonaws.com/onnx-model-zoo/resnet/resnet18v1/resnet18v1.onnx has problems between global average pool op and gemm op. Specifically, according to ONNX spec, the output of global avg pool retains the trailing trivial dimensions.

Output data tensor from pooling across the input tensor. Dimensions will be N x C x 1 x 1

Thus, the output of this operation in the context of resnet18 will be (N, C, 1, 1), whereas gemm expects 2-dimensional input as implied by the ONNX spec:

Compute Y = alpha * A' * B' + beta * C, where input tensor A has shape (M, K) or (K, M), input tensor B has shape (K, N) or (N, K),

Clearly putting gemm right after global average pool does not conform to the spec. Either model or the spec should be changed.

@tjingrant
Copy link
Author

cc @ankkhedia @abhinavs95

@tjingrant
Copy link
Author

While we are on this subject, would you consider giving it a model name that is better than "main" (which is the current name)? This could be very helpful.

@Roshrini
Copy link

Roshrini commented Aug 2, 2018

@tjingrant If you see the python notebook used to generate the above resnet18 model, https://github.com/onnx/models/blob/master/models/image_classification/resnet/train_resnet.ipynb
you can see that it uses "Dense"(FullyConnected) layer after GlobalAveragePool. As FullyConnected operator is not present in ONNX, it was mapped to "Gemm" operator.(https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/contrib/onnx/mx2onnx/_op_translations.py#L211)

@ankkhedia You can regenerate the model to change the model name from "main" to "mxnet_converted_model"(https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/contrib/onnx/mx2onnx/export_onnx.py#L339)

@tjingrant
Copy link
Author

@Roshrini

Clearly putting gemm right after global average pool does not conform to the spec.

This issue is about nonconformity. The model in question is incompatible with the ONNX spec and the relevant spec was cited in my original issue content.

Note that I'm not claiming the model is semantically incorrect.

@vandanavk
Copy link

This should be fixed with PR apache/mxnet#13356.

@tjingrant
Copy link
Author

@vandanavk seems like the model in the model zoo is not yet updated.

@ankkhedia
Copy link
Contributor

Hi @tjingrant I have updated the model in the model zoo.Could you please verify it?
I am closing the issue as the models have bene fixed.

Please feel free to re-open in case issue persists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants