Complete pytorch grid_sample

[ziqiangxu8457]

Complete pytorch grid_sample

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

@gromero @masahi

[ziqiangxu8457]

Hi, @gromero
I have correct my PR's format, but what the tvm-ci/pr-merge error means?

[gromero]

Hi, @gromero I have correct my PR's format, but what the tvm-ci/pr-merge error means?

Hi @shukun-ziqiangxu . In this case the tvm-ci/pr-merge error points to a lint error in your commit. When that happens the linter shows you a diff with the change that must be applied to your code so the lint error is fixed, in that case:

[2022-03-06T17:31:36.897Z] --- tests/python/frontend/pytorch/test_forward.py	2022-03-06 17:29:46.847569 +0000
[2022-03-06T17:31:36.897Z] +++ tests/python/frontend/pytorch/test_forward.py	2022-03-06 17:31:35.913957 +0000
[2022-03-06T17:31:36.897Z] @@ -4176,11 +4176,11 @@
[2022-03-06T17:31:36.897Z]              return torch.nn.functional.grid_sample(
[2022-03-06T17:31:36.897Z]                  input=x,
[2022-03-06T17:31:36.897Z]                  grid=y,
[2022-03-06T17:31:36.897Z]                  mode=self._method,
[2022-03-06T17:31:36.897Z]                  padding_mode=self._padding_mode,
[2022-03-06T17:31:36.897Z] -                align_corners=self._align_corners
[2022-03-06T17:31:36.897Z] +                align_corners=self._align_corners,
[2022-03-06T17:31:36.897Z]              )
[2022-03-06T17:31:36.897Z]  
[2022-03-06T17:31:36.897Z]      methods = ["nearest", "bilinear", "bicubic"]
[2022-03-06T17:31:36.897Z]      padding_modes = ["zeros", "border", "reflection"]
[2022-03-06T17:31:36.897Z]      align_corners = [True, False]

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 align_corners=self._align_corners parameter that needs to get added to satisfy the linter.

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:

$ docker/bash.sh tlcpack/ci-lint:v0.68 ./tests/scripts/task_lint.sh

After you fix it you can amend (git commit --amend) and git push --force to the same branch you used to create the PR (i.e https://github.com/shukun-ziqiangxu/tvm/tree/shukun-xzq-main) or just commit the fix on top of the 22d7c19 and do a git push without any force. Either way the CI will be retriggered in the existing PR (no need to create an additional PR) and the CI tests will be reinitialized with your lint fix.

[gromero]

@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 git log to see other examples).

Also:

1. s/grid_sample() support various/grid_sample() supports various/
2. s/interploation/interpolation/ in all the places where the typo exists
3. s/TVM only support/TVM only supports/
4. Please start the commit tile title with an upper case, e.g.: "Complete..."

Thanks.

[ziqiangxu8457]

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.

[gromero]

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?

@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 script (thanks a lot @driazati !) which is ideal to reproduce issues caught by the CI locally. For example, if one wants to run locally the test_tune_rpc_tracker_parsing test, it's simply by:

$ ./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.

[ziqiangxu8457]

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?

@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 script (thanks a lot @driazati !) which is ideal to reproduce issues caught by the CI locally. For example, if one wants to run locally the test_tune_rpc_tracker_parsing test, it's simply by:

$ ./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.

@gromero Thanks to your comment, I have fixed the "docs" warning. Please review my changes. Thanks a lot.

[gromero]

@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?

[masahi]

Amazing!

[masahi]

This is not pytorch test

[masahi]

This is not pytorch test

[gromero]

BTW, "Co-authored-by: shukun.net" in my commit is our company's commit rule for open source library like TVM, because the shukun info in my account is not noticeable, and the user account is inconstant. TVM is our important optimization tool for DL models, we will keep making contributions to TVM. Thanks for your understanding.

@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 Co-authored-by: Ziqiang XU <xuzq1@shukun.net> work for your employer's purpose?

I'm asking that because the format Co-authored-by: shukun.net is inappropriate.

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 git log):

Author: Ziqiang XU <xuzq1@shukun.net>
Date:   Thu Mar 3 01:34:52 2022 +0800
<snip>

If the PR is from multiple authors working for Shukun, then could you please list each of them in a separate Co-authored-by: per line entry providing their respective names and emails?

[gromero]

@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.

[gromero]

@shukun-ziqiangxu Thank for your contribution! @masahi Thanks for the review. It's now merged.

[ziqiangxu8457]

BTW, "Co-authored-by: shukun.net" in my commit is our company's commit rule for open source library like TVM, because the shukun info in my account is not noticeable, and the user account is inconstant. TVM is our important optimization tool for DL models, we will keep making contributions to TVM. Thanks for your understanding.

@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 Co-authored-by: Ziqiang XU <xuzq1@shukun.net> work for your employer's purpose?

I'm asking that because the format Co-authored-by: shukun.net is inappropriate.

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 git log):

Author: Ziqiang XU <xuzq1@shukun.net>
Date:   Thu Mar 3 01:34:52 2022 +0800
<snip>

If the PR is from multiple authors working for Shukun, then could you please list each of them in a separate Co-authored-by: per line entry providing their respective names and emails?

@gromero
Sorry for my late reply.

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.