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/21 17:36:45 UTC

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

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