mbrookhart edited a comment on issue #5662:
URL: https://github.com/apache/incubator-tvm/issues/5662#issuecomment-632992650


   Hey Cody. 
   
   This might just be a design misalignment. When writing the partition 
function, I explicitly dropped the constants from the inputs because I assumed 
we'd want to propagate constants through the body of the function. 
   
   If that was an incorrect assumption, this change:
   ```
   diff --git a/src/relay/ir/dataflow_matcher.cc 
b/src/relay/ir/dataflow_matcher.cc
   index 980935c34..f89116e34 100644
   --- a/src/relay/ir/dataflow_matcher.cc
   +++ b/src/relay/ir/dataflow_matcher.cc
   @@ -544,7 +544,7 @@ class PatternGrouper : protected MixedModeVisitor {
              auto matches = node_map[node->ref_];
              for (auto match : matches) {
                if (fuzzy_matches.count(match) == 0 && match.as<OpNode>() == 
nullptr &&
   -                match.as<FunctionNode>() == nullptr && 
match.as<ConstantNode>() == nullptr) {
   +                match.as<FunctionNode>() == nullptr) {
                  inputs[match] = Var(
                      "FunctionVar_" + std::to_string(graph_number_) + "_" + 
std::to_string(var_number),
                      NullValue<Type>());
   ```
   
   Gives me this result:
   ```
   === Fuse ====
   free_var %x: Tensor[(1, 3, 224, 224), float32]
   free_var %b: Tensor[(3), float32]
   %1 = fn (%p0: Tensor[(1, 3, 224, 224), float32], %p1: Tensor[(3, 3, 3, 3), 
float64], %p2: Tensor[(3), float32], Primitive=1) -> Tensor[(1, 3, 222, 222), 
float32] {
     %0 = nn.conv2d(%p0, %p1, padding=[0, 0, 0, 0]) /* ty=Tensor[(1, 3, 222, 
222), float32] */;
     nn.bias_add(%0, %p2) /* ty=Tensor[(1, 3, 222, 222), float32] */
   };
   %1(%x, meta[relay.Constant][0] /* ty=Tensor[(3, 3, 3, 3), float64] */ /* 
ty=Tensor[(3, 3, 3, 3), float64] */, %b) /* ty=Tensor[(1, 3, 222, 222), 
float32] */
   // meta data omitted. you can use show_meta_data=True to include meta data
   === Partition ===
   free_var %x: Tensor[(1, 3, 224, 224), float32]
   free_var %b: Tensor[(3), float32]
   %1 = fn (%FunctionVar_0_0, %FunctionVar_0_1, %FunctionVar_0_2, 
Composite="aa", PartitionedFromPattern="nn.conv2d_nn.bias_add_") {
     %0 = nn.conv2d(%FunctionVar_0_0, %FunctionVar_0_1, padding=[0, 0, 0, 0]);
     nn.bias_add(%0, %FunctionVar_0_2)
   };
   %1(%x, meta[relay.Constant][0] /* ty=Tensor[(3, 3, 3, 3), float64] */ /* 
ty=Tensor[(3, 3, 3, 3), float64] */, %b)
   // meta data omitted. you can use show_meta_data=True to include meta data
   ```
   
   Where the only differences I see is the naming of function variables and the 
fact that partition doesn't do type inference by default. 
   
   Do you think that's the correct behavior? It breaks a couple of other unit 
tests, I'll see if I can fix them.


----------------------------------------------------------------
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]


Reply via email to