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 2021/12/17 07:32:03 UTC
[GitHub] [tvm] shengxinhu opened a new pull request #9767: [Relay] Optimize dataflow partition to only check expr in current group
shengxinhu opened a new pull request #9767:
URL: https://github.com/apache/tvm/pull/9767
I think current check_ function in PatternPartitioner should only check exprs in current matched group, exclude of exprs outside of the matched group(these exprs are deemed as args of current group). For example, if a target has a very special requirement for nn.pad, such as requires pad_value equal to 0. we has defined thousands of patterns for the target and nn.pad exists in many of them. Current check_ function is impossible to distinguish nn.pad in current group from nn.pad in args of current group, but I only care about whether nn.pad in current group is valid for the target.
--
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] mbrookhart commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1030124694
@shengxinhu I don't think I'm really understanding your use case. Perhaps we can move this to a higher bandwidth conversation, say on the TVM Community discord? Send me a direct message, my handle is mbrookhart there 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.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1013537599
@shengxinhu I'm so sorry, I somehow missed this when you pinged me. I'm approaching end of week here, but I'm throwing this on my TODO list for first thing next week.
I'm trying to remember what I did here, this was like 18 months ago now. I think my inention was to allow constants to be used across groups, even if nodes couldn't be. While I'm digging into what's going on here, would it be possible to give me a small testcase for the issue you're hitting with pad?
--
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] shengxinhu closed pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu closed pull request #9767:
URL: https://github.com/apache/tvm/pull/9767
--
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] shengxinhu commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-997549985
Hi @mbrookhart , there are many test cases could not pass this changes, as there are new Variables in new function body. Do you has any better idea for my issue? or maybe I need to open a discussion for the issue.
--
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] mbrookhart commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1032931537
Ah, I think I understand now. I see a couple of options.
The simplest might be to run this pass on the graph before doing pattern matching, that should remove most of the pads, but it would also require removing the pads from your patterns.
https://github.com/apache/tvm/blob/824772489e514faf06025d7c09ce4dc13dcd7d08/python/tvm/relay/transform/transform.py#L1214-L1224
The next best option might be to align the rewrite callback and check APIs. For Rewrite, we have the callback take the pre-rewrite grap, the post-rewrite graph, and the map of pattern->node matches:
https://github.com/apache/tvm/blob/824772489e514faf06025d7c09ce4dc13dcd7d08/src/relay/ir/dataflow_matcher.cc#L821
If we adjusted the `check` API to take the same arguments, that might solve the problem. Again, it would require adjusting definitions in other parts of the codebase.
--
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] shengxinhu commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1021823370
>
--
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] shengxinhu commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1021832403
> @shengxinhu I'm so sorry, I somehow missed this when you pinged me. I'm approaching end of week here, but I'm throwing this on my TODO list for first thing next week.
>
> I'm trying to remember what I did here, this was like 18 months ago now. I think my inention was to allow constants to be used across groups, even if nodes couldn't be. While I'm digging into what's going on here, would it be possible to give me a small testcase for the issue you're hitting with pad?
Hi mbrookhart , sorry for late response. Currently, check_ function in PatternPartitione must be mapped to pattern 1:1, one check_function only works for one pattern, as it needs to remember and traverse all ops in the pattern. Our target has thousands of pattern, but with several limitations: such as nn.pad with pad_value = 0. We want to use only one check_function for all patterns, which visit all sub ops in the root group expr recursively. However, the group root expr has sub ops not in the group, it can't know whether sub ops in pattern or not,. So I suggest to pass group function body in the check_function instead of root group expr. However, many tests use origin design, which is not compatible. Do you has any suggestions?
--
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] shengxinhu commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1033278763
@mbrookhart , thanks for your suggestions. I indeed believe that the check function should only check post-rewrite pattern function body, as it only servers for the pattern. It is worth to adjust other parts of the codebase.
--
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] shengxinhu commented on pull request #9767: [PartitionPattern] Modify Pattern Partitioner to only check expr in current group
Posted by GitBox <gi...@apache.org>.
shengxinhu commented on pull request #9767:
URL: https://github.com/apache/tvm/pull/9767#issuecomment-1032124903
@mbrookhart , I have open discussion at: https://discuss.tvm.apache.org/t/proposal-for-pattern-partition-improvement/11748. Is this what you mean TVM Community discord?
--
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