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/07/01 17:13:24 UTC
[GitHub] [incubator-tvm] ANSHUMAN87 opened a new pull request #5974: [Arith] Constant cancellation added for subtraction
ANSHUMAN87 opened a new pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974
Additional constant cancellation added with few useful test cases.
cc @tqchen , @yzhliu
----------------------------------------------------------------
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] ANSHUMAN87 commented on pull request #5974: [Arith] Constant cancellation added for subtraction
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974#issuecomment-652639159
Okay I got the point. Thanks!
So will it be correct to assume that the cases which are not handled by canonicalization should be added as rule in rewrite or it is other way around?
I am sorry, please bear with me in understanding it, as I found many discrepancies when I am using both together in simplify() .
----------------------------------------------------------------
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 #5974: [Arith] Constant cancellation added for subtraction
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974#issuecomment-652562410
The canonicalization based rules should already be sufficient for these cases, so if there is no explicit motivating usecases, it would be useful to keep things simple and not including the additional change
----------------------------------------------------------------
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 #5974: [Arith] Constant cancellation added for subtraction
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974#issuecomment-652599386
We are using the two systems in a complementary way. Given that the simplifier is a core piece of the system and can easily introduce regressions, it is preferred to be careful and only make changes when we find a concrete motivating cases.
----------------------------------------------------------------
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 #5974: [Arith] Constant cancellation added for subtraction
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974#issuecomment-652642206
yes, it would be great to also backed by a motivating example(e.g. the need to simplify indices for conv2d operator). Since we are not building a generic simplifier that resolves all complications. Closing this for now
----------------------------------------------------------------
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] ANSHUMAN87 commented on pull request #5974: [Arith] Constant cancellation added for subtraction
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #5974:
URL: https://github.com/apache/incubator-tvm/pull/5974#issuecomment-652566834
@tqchen : Thanks for your clear feedback! I did also have similar concerns here. These are some trivial issues I observed during my analysis. Now I understand canonicalization will resolve these.
But keeping that in mind, I found some redundant rules still exists in rewrite simplify.
So as you see, the clear distinction is missing.
So if it is possible, would you please help me understand what kind of rules should be part of rewrite simplify and which should be handled in canonical simplify.
This will help me put my future PR in right direction. TIA!
----------------------------------------------------------------
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