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 2020/06/27 01:18:41 UTC
[GitHub] [incubator-tvm] weberlo opened a new pull request #5940: Add Quantize/Dequantize Partitioning
weberlo opened a new pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940
Implements step 1 of the [Improvements to Automatic Quantization for Bare-Metal RFC](https://discuss.tvm.ai/t/rfc-improvements-to-automatic-quantization-for-bare-metal/7108).
The code still needs more thorough documentation, and I'm gonna wait to bake the visitors into C++ until we get some design feedback (either here or on the RFC).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-674450371
@tmoreau89 @ZihengJiang CI's finally passing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-672282100
@ZihengJiang I've rebased and converted the datatype collector pass into C++, but I won't be prioritizing C++ conversion for the other visitors for the next few weeks. To prevent bit rot, I think we should work towards merging the PR in its current state.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen edited a comment on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-671440740
@weberlo please rebase to resolve the conflict. @ZihengJiang please help to manage this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-672323092
> @weberlo Will this affect the original pipeline when the config `partition_conversions` is `disabled`?
@ZihengJiang It won't affect the original pipeline, since we just return the `mod` [here](https://github.com/apache/incubator-tvm/pull/5940/files#diff-0ca944afca91e5a6d46981a4ac9e3dd6R378) when `partition_conversions` is `disabled`.
> Nice work @weberlo, do can we add a partition_conversions=disabled unit test as well?
It's implicitly tested in [this helper function](https://github.com/apache/incubator-tvm/pull/5940/files#diff-c82103045cf6ee84456fa38e019e074fR143-R146). I explicitly set it to `disabled` to make it clearer in the most recent commit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] ZihengJiang commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
ZihengJiang commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-672284273
@weberlo Will this affect the original pipeline when the config `partition_conversions` is `disabled`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-671440740
@weberlo please rebase to resolve the conflict. @ZihengJiang please help manage this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-673580414
> @weberlo I believe we would like a general approach to achieve the partitioning in the future, but I am fine with the PR in the current stage. I will approve it after it pass the tests.
I agree the current implementation is suboptimal, and it would be much better to integrate it more closely with the quantization pass itself. I went with the approach in this PR, because you're in the process of making significant changes to quantization, and this approach should be robust to those changes. We should certainly revisit this feature once your changes land though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] ZihengJiang commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
ZihengJiang commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-673134312
@weberlo I believe we would like a general approach to achieve the partitioning in the future, but I am fine with the PR in the current stage. I will approve it after it pass the tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] ZihengJiang commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
ZihengJiang commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-674450450
Merged now! Thanks Logan!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo edited a comment on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo edited a comment on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-673580414
> @weberlo I believe we would like a general approach to achieve the partitioning in the future, but I am fine with the PR in the current stage. I will approve it after it pass the tests.
@ZihengJiang I agree the current implementation is suboptimal, and it would be much better to integrate it more closely with the quantization pass itself. I went with the approach in this PR, because you're in the process of making significant changes to quantization, and this approach should be robust to those changes. We should certainly revisit this feature once your changes land though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tmoreau89 commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-672314270
Nice work @weberlo, do can we add a `partition_conversions=disabled` unit test as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] weberlo commented on pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
weberlo commented on pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940#issuecomment-652652538
CC @ZihengJiang
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tmoreau89 merged pull request #5940: Add Quantize/Dequantize Partitioning
Posted by GitBox <gi...@apache.org>.
tmoreau89 merged pull request #5940:
URL: https://github.com/apache/incubator-tvm/pull/5940
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org