comaniac commented on a change in pull request #5320: [BYOC] Prevent duplicate
outputs in subgraph Tuple
URL: https://github.com/apache/incubator-tvm/pull/5320#discussion_r407706932
##########
File path: tests/python/relay/test_annotated_regions.py
##########
@@ -56,7 +56,7 @@ def test_region_set_creator_diamond():
'test_target',
[cb_1],
[cb_1, O_1, ce_1, ce_2],
- [ce_1, ce_2],
Review comment:
This is a valuable question. For the use case of partition graph pass,
duplicated outputs are definitely not required. However, for the semantic of
`AnnotatedRegionSet` itself, it seems weird to find 2 boundary `compiler_ends`
in node list but only one of them is marked as an output. Accordingly, I have 3
proposals:
A. Let `AnnotatedRegionSet` maintain unique outputs as this PR. We may add
some comments mentioning that if there are duplicated `compiler_end`s in a
region, we will randomly mark one of them as an output.
B. `AnnotatedRegionSet` still keeps all outputs, but provides another API
(e.g., `GetUniqueOutputs()`) that contains the de-duplication logic in this PR.
`GetUniqueOutputs` returns another private class member `unique_outs_` which
will be updated every time `outs_` is updated. It also means that `outs_` must
be a private member as well and can only be updated via APIs.
C. `AnnotatedRegionSet` does not care about duplicated outputs. It will mark
all `compiler_end`s at boundary as outputs. In this case, `PartitionGraph`
needs to maintain a cache for region outputs.
cc @trevor-m @zhiics @mbaret @manupa-arm
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
