[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-587124344 @zhiics I merged your changes and updated the branch. Would you mind taking another look? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586541787 It seems like we need to flip the args. I’ll open the issue on Monday. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586537875 Alright, sorry for all of the spam. All tests are now fixed. cc @zhiics @mbarrett97 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586529239 This test is now failing at the de-duplicate pass. It's interesting since the "correct" graph has the exact same function duplicated twice. Whereas the "expected" graph is just re-using the same function. What should be the correct behavior? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527973 Definitely, I'll do that after fixing this. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527594 Thanks everyone for working through this with me! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586527376 This is actually a good exercise, as some other tests are actually failing now due to the graphs not being the same. For example, `test_branch_merge`. @mbarrett97, I'll push the changes, let me know what you think. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586525810 If I flip the arguments to alpha_equals and properly add the attributes to the "expected" function, the tests work as expected. With everyone's okay, can I go ahead with this 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586521847 Actually, are you sure the second argument is expected? It looks like AlphaEqual loops through the LHS args. This has some weird implications, when I run the other tests and flip the order of `result` and `expected` in alpha_equal, they fail. Maybe this is because the "expected" doesn't have the composite and primitive attributes? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586515076 I think so. If everyone else is okay, I think it's best to check this fix in. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586512825 Sorry, what exactly do you mean? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586509270 That's really strange - it looks right. Are you able to pull my branch and give the test a try? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586472422 I'll do some investigation :) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586469382 graph_equal also succeeds on the buggy graph :( @zhiics, let me know when you have any findings! 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586428127 Yes, they both pass the alpha_equal check. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call
node in MergeComposite pass
URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586412142
@mbarrett97 @comaniac I just pushed a test where "result" creates an
incorrect graph, and "expected" is correct. Even those these two graphs are
different, and "result" generated an incorrect function, they are
computationally equivalent. This means that the test actually passes
`alpha_equal` both with and without the bug. Here are the Relay outputs. The
pattern I am trying to match is `add -> add -> add`.
Result:
```
v0.0.4
fn (%a: Tensor[(10, 10), float32], %b: Tensor[(10, 10), float32]) ->
Tensor[(10, 10), float32] {
%0 = subtract(%a, %b) /* ty=Tensor[(10, 10), float32] */;
%4 = fn (%x: Tensor[(10, 10), float32], %y: Tensor[(10, 10), float32],
Primitive=1, Composite="add_add_add") -> Tensor[(10, 10), float32] {
%1 = add(%x, %y) /* ty=Tensor[(10, 10), float32] */;
%2 = add(%x, %1) /* ty=Tensor[(10, 10), float32] */;
%3 = add(%x, %y) /* ty=Tensor[(10, 10), float32] */;
add(%2, %3) /* ty=Tensor[(10, 10), float32] */
};
%4(%0, %b) /* ty=Tensor[(10, 10), float32] */
}
```
Expected:
```
v0.0.4
fn (%a: Tensor[(10, 10), float32], %b: Tensor[(10, 10), float32]) ->
Tensor[(10, 10), float32] {
%0 = subtract(%a, %b) /* ty=Tensor[(10, 10), float32] */;
%3 = fn (%in_1: Tensor[(10, 10), float32], %in_2: Tensor[(10, 10),
float32]) -> Tensor[(10, 10), float32] {
%1 = add(%in_1, %in_2) /* ty=Tensor[(10, 10), float32] */;
%2 = add(%in_1, %1) /* ty=Tensor[(10, 10), float32] */;
add(%2, %1) /* ty=Tensor[(10, 10), float32] */
};
%3(%0, %b) /* ty=Tensor[(10, 10), float32] */
}
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass
soiferj commented on issue #4879: [Relay][Pass] Fix bug in re-processing call node in MergeComposite pass URL: https://github.com/apache/incubator-tvm/pull/4879#issuecomment-586044128 Sure, I'll work on adding a unit test. 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: us...@infra.apache.org With regards, Apache Git Services
