-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Complete pytorch grid_sample #10504
Complete pytorch grid_sample #10504
Conversation
Hi, @gromero |
Hi @shukun-ziqiangxu . In this case the
Last lines from: https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-10504/1/pipeline In that case the error is that a comma is missing after the You can also run the linter locally (which is usually faster than the CI run, so I recommend you running the linter locally to check your code before submitting it to the CI) by the following command executed from TVM's root tree:
After you fix it you can amend ( |
@shukun-ziqiangxu When pushing the fix for the CI, please fix the commit message too: Delete "This contribution comes from @shukun.net" text since that info should come from the email in the commit. We usually don't use statements like that in the commit body also. If another person besides you is contributing the changes, please use the "Co-authored-by:" tag (see example in 5e353d5 or Also:
Thanks. |
64d66cf
to
b53e6f6
Compare
5cce73b
to
722c491
Compare
Hi, @gromero Sorry to bother you again. I have fixed all errors except the docs:GPU warning "Inline interpreted text or phrase reference start-string without end-string". I have tried several ways, but the warning still confuses me. Would you help me to fix it? Thanks. |
12fd31e
to
3c3e04e
Compare
@shukun-ziqiangxu Really sorry for missing your comment :( I have to improve the GH filters on my side I guess. It's not bothering by any means! I just saw you've pushed a new series and CI is running. I'll keep an eye on it. BTW, I've just got aware of the $ ./tests/scripts/ci.py cpu --tests tests/python/driver/tvmc/test_autotuner.py::test_tune_rpc_tracker_parsing This can really speed up debugging issues reported by the CI. |
f3ae7ac
to
435f094
Compare
435f094
to
0d526e5
Compare
Pytorch's grid_sample() supports various interpolation options: (1) data dimension: 2D / 3D (2) interpolation method: nearest / bilinear / bicubic (3) padding_mode: zeros / border / reflection (4) align_corners: True / False However, TVM only supports a part of above options: (1) data dimension: 2D (2) interpolation method: bilinear (3) padding_mode: zeros / border (4) align_corners: True This commit completes the options not supported by TVM, and keeps existing grid_sample of onnx/pytorch uninfluenced. Co-authored-by: shukun.net
0d526e5
to
c6dedde
Compare
@gromero Thanks to your comment, I have fixed the "docs" warning. Please review my changes. Thanks a lot. |
@masahi I did a first pass on this and it looks alright, but I'm not totally confident I can review it thoroughly. Now that tests passed, could we get reviews from you? |
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.
Amazing!
for _align in align_corners: | ||
verify_grid_sample(data_2D_shape, grid_2D_shape, _method, _padding, _align) | ||
|
||
# 3D "bicubic"(tricubic) is not supported in pytorch |
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 not pytorch test
data_2D_shape, grid_2D_shape, _method, layout_2D, _padding, _align | ||
) | ||
|
||
# 3D "bicubic"(tricubic) is not supported in pytorch |
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 not pytorch test
@shukun-ziqiangxu Hi. I just would like to clarify what's the idea you're trying to convey here before I merge this PR. Is this PR from a single author or from multiple authors? If it's from a single author (I'm assuming in that case that the author is you, XU) won't I'm asking that because the format Finally, I could not follow why the email found in the commit header does not work as a evidence that you're contribution from Shukun if it reads (from
If the PR is from multiple authors working for Shukun, then could you please list each of them in a separate |
@shukun-ziqiangxu Actually, since we don't have a guideline for that tag in TVM currently, it must not be a blocker for the PR. |
@shukun-ziqiangxu Thank for your contribution! @masahi Thanks for the review. It's now merged. |
@gromero I have come to realize that this commit header format is inappropriate. Thanks for your patience tips, I have already known how to do it better. In the future contributions, I will make my PRs in more appropriate way. Thanks a lot. |
Pytorch's grid_sample() supports various interpolation options: (1) data dimension: 2D / 3D (2) interpolation method: nearest / bilinear / bicubic (3) padding_mode: zeros / border / reflection (4) align_corners: True / False However, TVM only supports a part of above options: (1) data dimension: 2D (2) interpolation method: bilinear (3) padding_mode: zeros / border (4) align_corners: True This commit completes the options not supported by TVM, and keeps existing grid_sample of onnx/pytorch uninfluenced. Co-authored-by: shukun.net
Pytorch's grid_sample() supports various interpolation options: (1) data dimension: 2D / 3D (2) interpolation method: nearest / bilinear / bicubic (3) padding_mode: zeros / border / reflection (4) align_corners: True / False However, TVM only supports a part of above options: (1) data dimension: 2D (2) interpolation method: bilinear (3) padding_mode: zeros / border (4) align_corners: True This commit completes the options not supported by TVM, and keeps existing grid_sample of onnx/pytorch uninfluenced. Co-authored-by: shukun.net
Complete pytorch grid_sample
@gromero @masahi