-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Set default value of p in LpPool as 2 #8866
Conversation
Looks like the added test case in this PR doesn't pass CI, can you try this test case locally to see if it can be reproduced? |
Fix bug in test.
Sorry, I incorrectly calculate the output shape of test input. It should be correct now. :) |
Thanks Luyao! It all looks good to me |
@luyaor CI failed for unrelated things, could you push again to retrigger CI? |
nvm I retriggered CI |
Looks like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the error on CI
E onnxruntime.capi.onnxruntime_pybind11_state.Fail: [ONNXRuntimeError] : 1 : FAIL : Node: Output:y [ShapeInferenceError] Can't merge shape info. Both source and target dimension have values but they differ. Source=32 Target=31 Dimension=2
Seems the previous output shape [1, 1, 32, 32] was correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the shape issue LGTM
update with correct shape.
updated. |
Updated. |
Co-authored-by: Wuwei Lin <vincentl13x@gmail.com>
Thanks @luyaor @vinx13 @junrushao1994 @jwfromm ! |
* main: [UnitTests][Contrib] Enable contrib tensorrt/coreml unit tests (apache#8902) [BUG] DataType Bug In SplitRel (apache#8899) Enable python debug runtime for exported network libraries (apache#8793) Set default value of p in LpPool as 2 (apache#8866) [Community] @Hzfengsy -> Committer (apache#8908) Trivial uTVM -> microTVM "spelling" fix to align with branding. (apache#8905) [Vulkan][Topi] Parametrizing additional topi tests, marking vulkan failures (apache#8904) Move to new style issue template system (apache#8898) [Onnx] Support Negative Log Loss (apache#8872) [ROCm][TVMC] Add ROCm to the TVMC driver (apache#8896) fix error report on Store (apache#8895) [Docker] Re-enabled automatic --tty flag when running bash. (apache#8861)
* Set default value of p in LpPool as 2 * Update test_forward.py Fix bug in test. * Update test_forward.py update with correct shape. * Update onnx.py * Update python/tvm/relay/frontend/onnx.py Co-authored-by: Wuwei Lin <vincentl13x@gmail.com> Co-authored-by: luyaor <luyaor@luyaordeMacBook-Pro.local> Co-authored-by: Wuwei Lin <vincentl13x@gmail.com>
* Set default value of p in LpPool as 2 * Update test_forward.py Fix bug in test. * Update test_forward.py update with correct shape. * Update onnx.py * Update python/tvm/relay/frontend/onnx.py Co-authored-by: Wuwei Lin <vincentl13x@gmail.com> Co-authored-by: luyaor <luyaor@luyaordeMacBook-Pro.local> Co-authored-by: Wuwei Lin <vincentl13x@gmail.com>
@jwfromm Sorry for very late update. Followed #7242.
I have already formatted the code based on #7242 (comment).
Hope to merge :)