-
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
[Frontend][PaddlePaddle] Add topk op and Fix bug, when the output is a dimension, it … #13701
Conversation
…should not be squeezed
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
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.
LGTM
return paddle.topk(inputs, k=1) | ||
|
||
input_data = paddle.to_tensor([1, 4, 5, 7]) | ||
verify_model(topk1, input_data=input_data) |
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.
You can add a few different cases, example:
- k is a tensor and not just a int
- set largest/axis/sort attribute
- The output is only the values or the indices.
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.
according to paddle.topk api, there seems is no a argument to control the output that only have the values or indices
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.
Yes, the paddle.topk api will definitely have two outputs.
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.
LGTM
Would you like to fix the CI so we could get it in? |
|
|
@heliqi I modified the Paddles version configuration of the above two location. but the following prompt information of code is mismatch 2.4.1 version of Paddle. Can you take a look at this question for
|
@junrushao |
Thank you for pinging me @woobinw! Indeed the failure is not relevant to the change you brought in, and I'm not sure if it's flaky or consistently reproducible error. Retriggered the test just now, and let's see if it could work this time |
@tvm-bot rerun |
Could you rebase to latest HEAD? I’m sure it’s going to pass this time |
Yes, It has been rebased to latest HEAD. |
seems like there is some issues in rebasing and now the PR has inflated to 25k lines |
Thanks for the contribution! It’s merged! |
…a dimension, it … (apache#13701) There are some operation in PaddlePaddle that output a tensor which shape is [1], meanwhile, the slice op have decrease_axis attr, but it should not be squeezed.
There are some operation in PaddlePaddle that output a tensor which shape is [1], meanwhile, the slice op have decrease_axis attr, but it should not be squeezed.
@heliqi
@jiangjiajun
cc @junrushao