ZhennanQin commented on a change in pull request #16131: Fix for duplicate
subgraph inputs/outputs
URL: https://github.com/apache/incubator-mxnet/pull/16131#discussion_r323525598
##########
File path: src/operator/subgraph/subgraph_property.h
##########
@@ -296,8 +296,20 @@ class SubgraphProperty {
*/
virtual void ConnectSubgraphOutputs(const nnvm::NodePtr subgraph_node,
std::vector<nnvm::NodeEntry*>*
output_entries) const {
+ // Collapse output_entries pointing to same NodeEntry
Review comment:
Of course each entries have their unique address, which means, they are
presenting different data consuming for this node's output. But, if some
entries are the same(same node, index and version), then they should still be
the 'same' after subgraph partition(same means the entry value is the same
among those entries, but of course the entry value is different from before
partition).
Take your example as example,
After partition, the result would be
```
nodes : [
{ name : "SubgraphA" }, #subgraph_nodes[0], have 2 outputs, the output[1]
presents the origin output of NodeA
{ name : "NodeB", #nodes[1]
inputs : [[0,1,0]] }, #pointing at SubgraphA, repsents the output data
require of NodeA
{ name : "NodeC", #nodes[2]
inputs : [[0,1,0]] } #pointing at SubgraphA, repsents the output data
require of NodeA
]
```
So, here's what we can do to simplify the work in `ConnectSubgraphOutputs`
1. Create a map about unique_entry -> origin entry address(Not necessary,
just for better performance).
2. Pass unique_entry to `ConnectSubgraphOutputs`, and return a map of
unique_entry -> unique_entry_after_partition.
3. Then we know, how each original entry should be replaced for. Then do the
replacement on all entry address based on above 2 maps.
What I what to do here is, we should avoid backend developer to handle the
duplicated entries, instead, backend should only tells the new connection after
partition. Take above case as example, backend developer should only tells that
the output data requirement on NodeA now should pointing to index 1 on new
subgraph node through `ConnectSubgraphOutputs`. And build_subgraph pass should
propagate this change on all original entries which points at NodeA.
----------------------------------------------------------------
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
