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 2020/12/17 22:55:03 UTC

[GitHub] [tvm] codeislife99 opened a new pull request #7126: Sparse fill empty rows op

codeislife99 opened a new pull request #7126:
URL: https://github.com/apache/tvm/pull/7126


   Thanks for contributing to TVM!   Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546891906



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       Ok, I understand now. Could you add a little more documentation to that effect? I would also fill the values associated invalid indices with zeros instead of -1. That way even if someone uses the indices, the effect would be nothing.
   
   Also, have you considered using dynamic shape instead? That would avoid the need for the negative indices.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547196976



##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+  return {topi::SparseFillEmptyRows(inputs[0], inputs[1], inputs[2], param->dense_shape)};
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr default_value,
+                             Array<Integer> dense_shape) {
+  auto attrs = make_object<SparseFillEmptyRowsAttrs>();
+  attrs->dense_shape = std::move(dense_shape);
+  static const Op& op = Op::Get("sparsefillemptyrows");
+  return Call(op, {sparse_indices, sparse_values, default_value}, Attrs(attrs), {});
+}
+
+TVM_REGISTER_GLOBAL("relay.op._make.sparsefillemptyrows").set_body_typed(MakeSparseFillEmptyRows);
+
+RELAY_REGISTER_OP("sparsefillemptyrows")
+    .describe(R"code(Return twice of normal addition of two tensors.

Review comment:
       Done

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);

Review comment:
       Done




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-749747412


   @comaniac Every time I select request changes on GitHub, it switches it "suggested changes". Maybe because I am not a committer?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-748921727


   cc : @trevor-m  @comaniac @zhiics 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 edited a comment on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 edited a comment on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-748921727


   cc : @trevor-m  @comaniac @zhiics PTAL 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547596393



##########
File path: include/tvm/topi/transform.h
##########
@@ -1386,6 +1386,96 @@ inline Array<Tensor> meshgrid(const Array<Tensor>& inputs, const std::string& in
   return result;
 }
 
+/*!
+ * \brief Fill Empty rows of a sparse tensor with default value
+ *
+ * \param sparse_indices Indices where values of the dense tensor exist
+ * \param sparse_values Values at the above indices respectively
+ * \param default_value Default value at to be used at empty rows
+ * \param dense_shape Dense shape of the sparse tensor
+ * \param name The name of the operation
+ * \param tag The tag to mark the operation
+ *
+ * \return A Tensor whose op member is the SparseFillEmptyRows operation
+ */
+inline Array<Tensor> SparseFillEmptyRows(const Tensor& sparse_indices, const Tensor& sparse_values,
+                                         const Tensor& default_value,
+                                         const Array<Integer>& dense_shape,
+                                         const std::string name = "T_sparse_fill_empty_rows",
+                                         std::string tag = kInjective) {
+  Array<Tensor> result;
+  Array<PrimExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  auto empty_row_indicator =
+      compute(Array<PrimExpr>{dense_shape[0]}, [&](const Array<Var>& indices) {
+        PrimExpr ret = PrimExpr(Bool(1));
+        for (int i = 0; i < GetConstInt(sparse_indices->shape[0]); ++i) {

Review comment:
       Yes there are 3 sparse ops that we are trying to target for a customer for a TF model explicitly. 
   These 3 are : 
   1. [sparse_reshape](https://www.tensorflow.org/api_docs/python/tf/sparse/reshape) 
   2. [sparse_segment_sum](https://www.tensorflow.org/api_docs/python/tf/sparse/segment_sum?hl=bn)
   3. [sparse_fill_empty_rows](https://www.tensorflow.org/api_docs/python/tf/sparse/fill_empty_rows)
   4. [sparse_segment_sum_sqrt_n](https://www.tensorflow.org/api_docs/python/tf/sparse/segment_sqrt_n?hl=bn)




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547197106



##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);

Review comment:
       Done.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);

Review comment:
       Done

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);

Review comment:
       Done

##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded

Review comment:
       Changed.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546836442



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded

Review comment:
       What is `to_be_discarded` it is not mentioned anywhere?

##########
File path: python/tvm/relay/op/_transform.py
##########
@@ -63,6 +63,7 @@
 _reg.register_injective_schedule("sparse_to_dense")
 _reg.register_injective_schedule("matrix_set_diag")
 _reg.register_injective_schedule("adv_index")
+_reg.register_injective_schedule("sparsefillemptyrows")

Review comment:
       I think `sparse_fill_empty_rows` fits better with our current naming convention.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+  return {topi::SparseFillEmptyRows(inputs[0], inputs[1], inputs[2], param->dense_shape)};
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr default_value,
+                             Array<Integer> dense_shape) {
+  auto attrs = make_object<SparseFillEmptyRowsAttrs>();
+  attrs->dense_shape = std::move(dense_shape);
+  static const Op& op = Op::Get("sparsefillemptyrows");
+  return Call(op, {sparse_indices, sparse_values, default_value}, Attrs(attrs), {});
+}
+
+TVM_REGISTER_GLOBAL("relay.op._make.sparsefillemptyrows").set_body_typed(MakeSparseFillEmptyRows);
+
+RELAY_REGISTER_OP("sparsefillemptyrows")
+    .describe(R"code(Return twice of normal addition of two tensors.

Review comment:
       Could you update the description.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);

Review comment:
       Can you add an error message here.

##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       I'm not sure this example is correct. `[-1, -1]` is not a valid sparse index. Shouldn't the output be
   ```
   new_sparse_indices=[[0, 1],
                                         [0, 3]
                                         [1, 0]
                                         [2, 0]
                                         [3, 1]
                                         [4, 0]]
   
   new_sparse_values = [1, 2, 10, 3, 4, 10]
   ``` 

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);

Review comment:
       Could you add an error message here. Something like "SparseFillEmptyRowsRel expects 4 arguments, but X were provided".

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);

Review comment:
       ```suggestion
     ICHECK(param != nullptr);
   ```

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,63 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4);
+  ICHECK_EQ(num_inputs, 3);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  CHECK_EQ(inputs.size(), 3);
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  CHECK(param != nullptr);

Review comment:
       ```suggestion
     ICHECK(param != nullptr);
   ```




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547597810



##########
File path: include/tvm/topi/transform.h
##########
@@ -1386,6 +1386,96 @@ inline Array<Tensor> meshgrid(const Array<Tensor>& inputs, const std::string& in
   return result;
 }
 
+/*!
+ * \brief Fill Empty rows of a sparse tensor with default value
+ *
+ * \param sparse_indices Indices where values of the dense tensor exist
+ * \param sparse_values Values at the above indices respectively
+ * \param default_value Default value at to be used at empty rows
+ * \param dense_shape Dense shape of the sparse tensor
+ * \param name The name of the operation
+ * \param tag The tag to mark the operation
+ *
+ * \return A Tensor whose op member is the SparseFillEmptyRows operation
+ */
+inline Array<Tensor> SparseFillEmptyRows(const Tensor& sparse_indices, const Tensor& sparse_values,
+                                         const Tensor& default_value,
+                                         const Array<Integer>& dense_shape,
+                                         const std::string name = "T_sparse_fill_empty_rows",
+                                         std::string tag = kInjective) {
+  Array<Tensor> result;
+  Array<PrimExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  auto empty_row_indicator =
+      compute(Array<PrimExpr>{dense_shape[0]}, [&](const Array<Var>& indices) {
+        PrimExpr ret = PrimExpr(Bool(1));
+        for (int i = 0; i < GetConstInt(sparse_indices->shape[0]); ++i) {

Review comment:
       I am a beginner in op implementation, do you mind sharing some PR/code examples , on how I can achieve the same goal(loop etc.) and set the output shape in `OpRel`. 
   Then I will follow them to make this compatible for dynamically shaped input tensors. 
   There is also probably no need for the strided_slice to be outside the op implementation if I change it to support dynamic shapes.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546888428



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       Let me know if you have any better suggestions for implementation, else I will stick with 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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 closed pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 closed pull request #7126:
URL: https://github.com/apache/tvm/pull/7126


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-749724064


   @tkonolige @mbrookhart PTAL and approve or request changes explicitly. Thanks.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r550161068



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       Resolving this conversation since based on our offline discussion, we will start off with static shape




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] comaniac commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-749772392


   > @comaniac Every time I select request changes on GitHub, it switches it "suggested changes". Maybe because I am not a committer?
   
   Yes that's the case, but we'll do our best to make sure all comments are addressed before merging.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547556384



##########
File path: include/tvm/topi/transform.h
##########
@@ -1386,6 +1386,96 @@ inline Array<Tensor> meshgrid(const Array<Tensor>& inputs, const std::string& in
   return result;
 }
 
+/*!
+ * \brief Fill Empty rows of a sparse tensor with default value
+ *
+ * \param sparse_indices Indices where values of the dense tensor exist
+ * \param sparse_values Values at the above indices respectively
+ * \param default_value Default value at to be used at empty rows
+ * \param dense_shape Dense shape of the sparse tensor
+ * \param name The name of the operation
+ * \param tag The tag to mark the operation
+ *
+ * \return A Tensor whose op member is the SparseFillEmptyRows operation
+ */
+inline Array<Tensor> SparseFillEmptyRows(const Tensor& sparse_indices, const Tensor& sparse_values,
+                                         const Tensor& default_value,
+                                         const Array<Integer>& dense_shape,
+                                         const std::string name = "T_sparse_fill_empty_rows",
+                                         std::string tag = kInjective) {
+  Array<Tensor> result;
+  Array<PrimExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  auto empty_row_indicator =
+      compute(Array<PrimExpr>{dense_shape[0]}, [&](const Array<Var>& indices) {
+        PrimExpr ret = PrimExpr(Bool(1));
+        for (int i = 0; i < GetConstInt(sparse_indices->shape[0]); ++i) {

Review comment:
       I would like to see this done in a way that supports dynamic shapes, this will crash a compile time if we have a dynamic input.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547596393



##########
File path: include/tvm/topi/transform.h
##########
@@ -1386,6 +1386,96 @@ inline Array<Tensor> meshgrid(const Array<Tensor>& inputs, const std::string& in
   return result;
 }
 
+/*!
+ * \brief Fill Empty rows of a sparse tensor with default value
+ *
+ * \param sparse_indices Indices where values of the dense tensor exist
+ * \param sparse_values Values at the above indices respectively
+ * \param default_value Default value at to be used at empty rows
+ * \param dense_shape Dense shape of the sparse tensor
+ * \param name The name of the operation
+ * \param tag The tag to mark the operation
+ *
+ * \return A Tensor whose op member is the SparseFillEmptyRows operation
+ */
+inline Array<Tensor> SparseFillEmptyRows(const Tensor& sparse_indices, const Tensor& sparse_values,
+                                         const Tensor& default_value,
+                                         const Array<Integer>& dense_shape,
+                                         const std::string name = "T_sparse_fill_empty_rows",
+                                         std::string tag = kInjective) {
+  Array<Tensor> result;
+  Array<PrimExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  auto empty_row_indicator =
+      compute(Array<PrimExpr>{dense_shape[0]}, [&](const Array<Var>& indices) {
+        PrimExpr ret = PrimExpr(Bool(1));
+        for (int i = 0; i < GetConstInt(sparse_indices->shape[0]); ++i) {

Review comment:
       Yes there are 3 sparse ops that we are trying to target for a customer for a TF model explicitly. 
   These 3 are : 
   1. [sparse_reshape](https://www.tensorflow.org/api_docs/python/tf/sparse/reshape) 
   2. [sparse_segment_sum](https://www.tensorflow.org/api_docs/python/tf/sparse/segment_sum?hl=bn)
   3. [sparse_fill_empty_rows](https://www.tensorflow.org/api_docs/python/tf/sparse/fill_empty_rows)




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547480430



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       Given you are using it for a single op, I would follow NMS's example and use dynamic shape.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546895484



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       No I haven't considered that. I noticed that for input dependent ops like `non_maximum_suppression`, they used nms with fixed size + strided slice and I just borrowed that idea. Do you mind pointing me to an implementation similar to what you are talking about ? 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r550279440



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,83 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparse_fill_empty_rows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+    It returns a TupleWrapper with four outputs
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at irrelevant indices
+        which will be sliced in a future op discarding non-useful elements. This is done since the
+        real rows of new_sparse_indices depends on the input.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices

Review comment:
       Done.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r547476827



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,83 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparse_fill_empty_rows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+    It returns a TupleWrapper with four outputs
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at irrelevant indices
+        which will be sliced in a future op discarding non-useful elements. This is done since the
+        real rows of new_sparse_indices depends on the input.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices

Review comment:
       Could you update the wording here? `to_be_discarded` refers to nothing.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  ICHECK_EQ(inputs.size(), 3) << "SparseFillEmptyRowsCompute expects 3 arguments but provided "
+                              << inputs.size();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);

Review comment:
       ```suggestion
     ICHECK_NOTNULL(param);
   ```

##########
File path: include/tvm/topi/transform.h
##########
@@ -1386,6 +1386,96 @@ inline Array<Tensor> meshgrid(const Array<Tensor>& inputs, const std::string& in
   return result;
 }
 
+/*!
+ * \brief Fill Empty rows of a sparse tensor with default value

Review comment:
       ```suggestion
    * \brief Fill empty rows of a sparse tensor with default values
   ```

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);

Review comment:
       ```suggestion
     ICHECK_NOTNULL(param);
   ```

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();

Review comment:
       ```suggestion
     ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but " << types.size() << " were provided.";
   ```

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  ICHECK_EQ(inputs.size(), 3) << "SparseFillEmptyRowsCompute expects 3 arguments but provided "
+                              << inputs.size();

Review comment:
       ```suggestion
     ICHECK_EQ(inputs.size(), 3) << "SparseFillEmptyRowsCompute expects 3 arguments but " << inputs.size() << " were provided.";
   ```

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  ICHECK_EQ(inputs.size(), 3) << "SparseFillEmptyRowsCompute expects 3 arguments but provided "
+                              << inputs.size();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+  return {topi::SparseFillEmptyRows(inputs[0], inputs[1], inputs[2], param->dense_shape)};
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr default_value,
+                             Array<Integer> dense_shape) {
+  auto attrs = make_object<SparseFillEmptyRowsAttrs>();
+  attrs->dense_shape = std::move(dense_shape);
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, default_value}, Attrs(attrs), {});
+}
+
+TVM_REGISTER_GLOBAL("relay.op._make.sparse_fill_empty_rows")
+    .set_body_typed(MakeSparseFillEmptyRows);
+
+RELAY_REGISTER_OP("sparse_fill_empty_rows")
+    .describe(R"code(Return representation of a sparse tensor with empty rows filled with default 
+    value.)code" TVM_ADD_FILELINE)
+    .set_num_inputs(3)
+    .set_attrs_type<SparseFillEmptyRowsAttrs>()
+    .add_argument("sparse_indices", "Tensor", "The first tensor")

Review comment:
       Please put more useful descriptions on these arguments.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546961924



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       Yeah, in the short-term it will be used for TF. https://www.tensorflow.org/api_docs/python/tf/sparse/fill_empty_rows




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546888139



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       So , if you look at the description (`N + dense_shape[0]` ) <-- this is the dim0 shape of the tensor. This is because we can't know beforehand how many rows are missing. This operation would be succeeded by strided_slice to remove with begin as 0 and end as `slice_element_index` to achieve the desired output 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-778546996


   Hoping back onto this stream of work after a while, was involved with something else. 
   Since this PR had a lot of issues, it has been completely rewritten in #7442 and addresses all the existing comments in this PR. This Op has also been tested E2E with the customer model. Please review #7442. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r550291956



##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1553,6 +1553,65 @@ RELAY_REGISTER_OP("meshgrid")
     .set_attr<FTVMCompute>("FTVMCompute", MeshgridCompute)
     .set_attr<TOpPattern>("TOpPattern", kInjective);
 
+TVM_REGISTER_NODE_TYPE(SparseFillEmptyRowsAttrs);
+
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [ sparse_indices, sparse_values, default_values, result]
+  ICHECK_EQ(types.size(), 4) << "SparseFillEmptyRowsRel expects 4 arguments but provided "
+                             << types.size();
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto default_value = types[2].as<TensorTypeNode>();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+
+  Array<IndexExpr> sp_ordered_output_shape;
+  sp_ordered_output_shape.push_back(param->dense_shape[0] + sparse_indices->shape[0]);
+  if (sparse_indices->shape.size() > 1) {
+    sp_ordered_output_shape.push_back(sparse_indices->shape[1]);
+  }
+  fields.push_back(TensorType(sp_ordered_output_shape, sparse_indices->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{param->dense_shape[0]}, tvm::DataType::Bool()));
+  fields.push_back(TensorType(Array<PrimExpr>{sp_ordered_output_shape[0]}, default_value->dtype));
+  fields.push_back(TensorType(Array<PrimExpr>{1}, tvm::DataType::Int(32)));
+  reporter->Assign(types[3], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Array<te::Tensor> SparseFillEmptyRowsCompute(const Attrs& attrs, const Array<te::Tensor>& inputs,
+                                             const Type& out_type) {
+  ICHECK_EQ(inputs.size(), 3) << "SparseFillEmptyRowsCompute expects 3 arguments but provided "
+                              << inputs.size();
+  const auto* param = attrs.as<SparseFillEmptyRowsAttrs>();
+  ICHECK(param != nullptr);
+  return {topi::SparseFillEmptyRows(inputs[0], inputs[1], inputs[2], param->dense_shape)};
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr default_value,
+                             Array<Integer> dense_shape) {
+  auto attrs = make_object<SparseFillEmptyRowsAttrs>();
+  attrs->dense_shape = std::move(dense_shape);
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, default_value}, Attrs(attrs), {});
+}
+
+TVM_REGISTER_GLOBAL("relay.op._make.sparse_fill_empty_rows")
+    .set_body_typed(MakeSparseFillEmptyRows);
+
+RELAY_REGISTER_OP("sparse_fill_empty_rows")
+    .describe(R"code(Return representation of a sparse tensor with empty rows filled with default 
+    value.)code" TVM_ADD_FILELINE)
+    .set_num_inputs(3)
+    .set_attrs_type<SparseFillEmptyRowsAttrs>()
+    .add_argument("sparse_indices", "Tensor", "The first tensor")

Review comment:
       Done.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#issuecomment-752721781


   @tkonolige @mbrookhart This PR is ready for re-review as well whenever you are back from vacation. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] mbrookhart commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546916764



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1320,3 +1320,84 @@ def adv_index(inputs):
         Output tensor.
     """
     return _make.adv_index(Tuple(inputs))
+
+
+def sparsefillemptyrows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    Fill first column of the empty rows with default values for a sparse array.
+
+    Parameters
+    ----------
+    sparse_indices : relay.Expr
+        A 2-D tensor[N, n_dim] of integers containing location of sparse values, where N is the
+        number of sparse values and n_dim is the number of dimensions of the dense_shape
+
+    sparse_values : relay.Expr
+        A 1-D tensor[N] containing the sparse values for the sparse indices.
+
+    dense_shape : relay.Expr
+        A list of integers. Shape of the dense output tensor.
+
+    default_value : relay.Expr
+        A 0-D tensor containing the default value for the remaining locations.
+        Defaults to 0.
+
+    Returns
+    -------
+    TupleWrapper with the following four outputs
+
+    new_sparse_indices : relay.Expr
+        A 2-D tensor[N + dense_shape[0], n_dim] of integers containing location of new sparse
+        indices where N is the number of sparse values. It is filled with -1 at to_be_discarded
+        indices.
+
+    empty_row_indicator : relay.Expr
+        A 1-D Boolean tensor[dense_shape[0]] indicating whether the particular row is empty
+
+    new_sparse_values : relay.Expr
+        A 1-D tensor[dense_shape[0]] containing the sparse values for the sparse indices. It is
+        filled with -1 at to_be_discarded indices.
+
+    slice_element_index : relay.Expr
+        A 1-D tensor containing the amount of elements in the sparse_indices and new_sparse_values
+        expression to be sliced in a future op discarding non-useful elements in new_sparse_indices
+        and new_sparse_values
+
+    Examples
+    -------
+
+    .. code-block:: python
+
+    sparse_indices = [[0, 1],
+                      [0, 3],
+                      [2, 0],
+                      [3, 1]]
+    sparse_values = [1, 2, 3, 4]
+    default_value = [10]
+    dense_shape = [5, 6]
+    new_sparse_indices, empty_row_indicator, new_sparse_values, slice_element_index =
+                        relay.sparsereshape(
+                        sparse_indices,
+                        sparse_values,
+                        prev_shape,
+                        new_shape)
+    new_sparse_indices =       [[0, 1],
+                               [0, 3],
+                               [2, 0],
+                               [3, 1],
+                               [1, 0],
+                               [4, 0],
+                               [-1, -1],

Review comment:
       We have it set up that way because that's how MXNet does it (TOPI's original opset was mostly cloned from MXNet), and MXNet is fairly conservative about static shapes. From a performance perspective, if the expectation is that we'll always use this with a dynamic strided slice after, I'd think it would be better to just emit a dynamic shape. If we have another usecase, however, I think this is fine.
   
   Do you have a framework level usecase in mind?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] codeislife99 commented on a change in pull request #7126: Sparse fill empty rows op

Posted by GitBox <gi...@apache.org>.
codeislife99 commented on a change in pull request #7126:
URL: https://github.com/apache/tvm/pull/7126#discussion_r546893836



##########
File path: python/tvm/relay/op/_transform.py
##########
@@ -63,6 +63,7 @@
 _reg.register_injective_schedule("sparse_to_dense")
 _reg.register_injective_schedule("matrix_set_diag")
 _reg.register_injective_schedule("adv_index")
+_reg.register_injective_schedule("sparsefillemptyrows")

Review comment:
       Changed. 




----------------------------------------------------------------
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:
users@infra.apache.org