You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/11/18 21:41:47 UTC
[GitHub] [tvm] Lunderberg opened a new pull request, #13438: [TVMScript] Improvements tvm.script.highlight
Lunderberg opened a new pull request, #13438:
URL: https://github.com/apache/tvm/pull/13438
- Automatically use "black" formatter if available.
- Allow overrides of pygmentize style based on environment variable `TVM_PYGMENTIZE_STYLE`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320563101
This PR started from finding that the `dracula` style shows up best in my terminal, and wanted to have a way to enable it as the default pygmentize for my environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1324024874
Good point on the `T.Select` optimizing out during unrolling, so it wouldn't be something with semantic meaning early on. That makes sense on having the flag to disable it the formatting. Looks like I initially added `black_format=True` as an argument to `cprint`, but not to the methods that call into it. All the `obj.show()` methods now also have a `black_format=True` argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
junrushao commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320830722
Some background: The reason that winograd conv2d has such a long chained `T.Select` is that it's used to represent A, B, G matrices, and by unrolling the corresponding loops in our scheduling, the `T.Select` is constant-folded into scalar constants. Therefore there is no performance degradation if scheduled properly.
On sugaring long chained `T.Select`: The usecases of sugaring `T.Select`, if it's only limited to constant matrices in winograd conv, then it's probably less common and I would say it may not be desirable because it requires users to understand a new syntax.
My two-cents: as long as we have a flag to turn it off, in `.show()` methods, I don't have a strong opinion if it defaults to on or off :-) Alternatively, when the IR is much longer (e.g. +20 extra lines after formatting), we could print out a doc, saying you may turn formatting off by tweaking the flag, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tvm-bot commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320558625
<!---bot-comment-->
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](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* No users to tag found in teams: `tvmscript` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg merged pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
Lunderberg merged PR #13438:
URL: https://github.com/apache/tvm/pull/13438
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320766706
Hmm, I'm also wondering on the representation of `T.select`. Both it and `T.if_then_else` are useful to use in nested chains, which are what causes the poor formatting. If a nested sequence of `N` select/if_then_else were sugared as a sequence of `2*N + 1` arguments, we could avoid having a lot of the structures that get poorly formatted. So the winograd TIR example, line 37 would be formatted into ~40 flat lines, instead of the ~80 heavily indented lines that it currently formats into.
```python
B[i, j] = T.Select(
i % 4 == 3 and j % 4 == 3,
T.float32(1),
i % 4 == 3 and j % 4 == 2,
T.float32(0),
i % 4 == 3 and j % 4 == 1,
T.float32(0),
i % 4 == 3 and j % 4 == 0,
T.float32(0),
i % 4 == 2 and j % 4 == 3,
T.float32(0),
i % 4 == 2 and j % 4 == 2,
T.float32(1),
i % 4 == 2 and j % 4 == 1,
T.float32(1),
i % 4 == 2 and j % 4 == 0,
T.float32(-1),
i % 4 == 1 and j % 4 == 3,
T.float32(-1),
i % 4 == 1 and j % 4 == 2,
T.float32(1),
i % 4 == 1 and j % 4 == 1,
T.float32(-1),
i % 4 == 1 and j % 4 == 0,
T.float32(0),
i % 4 == 0 and j % 4 == 3,
T.float32(0),
i % 4 == 0 and j % 4 == 2,
T.float32(0),
i % 4 == 0 and j % 4 == 1,
T.float32(0),
i % 4 == 0 and j % 4 == 0,
T.float32(1),
T.float32(0),
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
junrushao commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320733488
Using black is format is a great idea! On the other hand, sometimes we want to toggle it off for huge IRs, for example, the winograd TIR has two extremely long lines which we dont really need to read much into: https://gist.github.com/zxybazh/0019023b0796a55c9fadb12593852f84. Would love to get some thoughts from you which one is better: formatting by default or not?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg commented on pull request #13438: [TVMScript] Improvements tvm.script.highlight
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13438:
URL: https://github.com/apache/tvm/pull/13438#issuecomment-1320760159
Oof, yeah, looks like that one becomes pretty heavily indented with `black`, with several sections starting at indentations around line 60-80. It looks like it is specifically the nested function calls (here the chain of `T.select`), each of which adds indentation.
I think I'd still lean toward having the formatting enabled by default, since I think it improves readability on average, but it would be good to have a way to disable it when debugging cases where the readability is worsened. Maybe having the argument default be set by an environment variable, so that the formatting could be enabled/disabled across all calls as needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org