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 2021/02/15 18:52:04 UTC

[GitHub] [tvm] tkonolige commented on a change in pull request #7442: SparseFillEmptyRows Op

tkonolige commented on a change in pull request #7442:
URL: https://github.com/apache/tvm/pull/7442#discussion_r576357085



##########
File path: python/tvm/relay/op/transform.py
##########
@@ -1322,6 +1322,20 @@ def adv_index(inputs):
     return _make.adv_index(Tuple(inputs))
 
 
+def sparse_fill_empty_rows(sparse_indices, sparse_values, dense_shape, default_value):
+    """
+    This op exactly follows the documentation here:
+    https://www.tensorflow.org/api_docs/python/tf/sparse/fill_empty_rows

Review comment:
       Instead of linking to the tensorflow documentation, could you write out what this op does. Please include parameters, return values, and examples.

##########
File path: python/tvm/relay/op/_transform.py
##########
@@ -445,6 +465,44 @@ def argwhere_shape_func(attrs, inputs, out_ndims):
 _reg.register_shape_func("scatter_add", False, elemwise_shape_func)
 
 
+@script
+def _sparse_fill_empty_rows_shape_func(sparse_indices, dense_shape):
+
+    new_sparse_indices_shape = output_tensor((2,), "int64")
+    new_sparse_values_shape = output_tensor((1,), "int64")
+    empty_row_indicator_shape = output_tensor((1,), "int64")
+    num_dense_rows = int64(dense_shape[0])
+
+    if int64(sparse_indices.shape[0]) == int64(0):

Review comment:
       This shape function does not support dynamic input shapes? Are you planning on using this op after any dynamic operations?

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1584,6 +1584,47 @@ RELAY_REGISTER_OP("repeat")
     .set_attr<FTVMCompute>("FTVMCompute", RepeatCompute)
     .set_attr<TOpPattern>("TOpPattern", kBroadcast);
 
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [sparse_indices, sparse_values, dense_shape, default_value, result]
+  ICHECK_EQ(types.size(), 5);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto ndims = sparse_indices->shape[1];
+  fields.push_back(TensorType(Array<PrimExpr>{Any(), ndims}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  reporter->Assign(types[types.size() - 1], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr dense_shape,
+                             Expr default_value) {
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, dense_shape, default_value}, 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)

Review comment:
       ```suggestion
       .describe(R"code(Fill empty rows of a sparse tensor with a default value.)code" TVM_ADD_FILELINE)
   ```
   

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1584,6 +1584,47 @@ RELAY_REGISTER_OP("repeat")
     .set_attr<FTVMCompute>("FTVMCompute", RepeatCompute)
     .set_attr<TOpPattern>("TOpPattern", kBroadcast);
 
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [sparse_indices, sparse_values, dense_shape, default_value, result]
+  ICHECK_EQ(types.size(), 5);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto ndims = sparse_indices->shape[1];
+  fields.push_back(TensorType(Array<PrimExpr>{Any(), ndims}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  reporter->Assign(types[types.size() - 1], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr dense_shape,
+                             Expr default_value) {
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, dense_shape, default_value}, 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(4)
+    .add_argument("sparse_indices", "Tensor",
+                  "A 2-D int64 tensor of shape [N, ndims], which specifies the indices of the"
+                  "elements in the sparse tensor that contain nonzero values")
+    .add_argument("sparse_values", "Tensor",
+                  "A 1-D tensor[N] which supplies the values for each element in indices")
+    .add_argument("dense_shape", "Tensor",

Review comment:
       Dense shape is a bit of a misnomer. This is just the actual shape of the sparse tensor.

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name

Review comment:
       Where are you getting the undefined-variable warning? This seems like a dangerous lint to disable.

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")
+    new_sparse_values = output_tensor(new_sparse_values_shape, "int64")
+    empty_row_indicator = output_tensor(empty_row_indicator_shape, "int64")
+    idx = 0
+
+    if int64(sparse_indices.shape[0]) == int64(0):
+        for i in range(0, int64(new_sparse_indices_shape[0])):
+            new_sparse_indices[i, 0] = int64(i)
+            new_sparse_values[i] = default_value_
+            empty_row_indicator[i] = int64(1)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[i, k] = int64(0)
+
+        return (new_sparse_indices, new_sparse_values, empty_row_indicator)
+
+    else:
+        for i in range(0, int64(sparse_indices[0, 0])):
+            new_sparse_indices[idx, 0] = int64(i)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[idx, k] = int64(0)
+
+            new_sparse_values[idx] = default_value_
+            empty_row_indicator[i] = int64(1)
+            idx += 1
+
+        for i in range(0, int64(sparse_indices.shape[0])):
+            index = int64(sparse_indices[i, 0])
+            if i == 0:
+                new_sparse_indices[idx, 0] = index

Review comment:
       Also, this line can be removed if you start the loop below at 0.

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")

Review comment:
       Shouldn't the output type match the input type?

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1584,6 +1584,47 @@ RELAY_REGISTER_OP("repeat")
     .set_attr<FTVMCompute>("FTVMCompute", RepeatCompute)
     .set_attr<TOpPattern>("TOpPattern", kBroadcast);
 
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [sparse_indices, sparse_values, dense_shape, default_value, result]
+  ICHECK_EQ(types.size(), 5);

Review comment:
       Could you put an error message on this.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1584,6 +1584,47 @@ RELAY_REGISTER_OP("repeat")
     .set_attr<FTVMCompute>("FTVMCompute", RepeatCompute)
     .set_attr<TOpPattern>("TOpPattern", kBroadcast);
 
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [sparse_indices, sparse_values, dense_shape, default_value, result]
+  ICHECK_EQ(types.size(), 5);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto ndims = sparse_indices->shape[1];
+  fields.push_back(TensorType(Array<PrimExpr>{Any(), ndims}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  reporter->Assign(types[types.size() - 1], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr dense_shape,
+                             Expr default_value) {
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, dense_shape, default_value}, 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(4)
+    .add_argument("sparse_indices", "Tensor",

Review comment:
       Please document the format. Looking at the code, it seems as if the format is COO?

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")
+    new_sparse_values = output_tensor(new_sparse_values_shape, "int64")
+    empty_row_indicator = output_tensor(empty_row_indicator_shape, "int64")
+    idx = 0
+
+    if int64(sparse_indices.shape[0]) == int64(0):
+        for i in range(0, int64(new_sparse_indices_shape[0])):
+            new_sparse_indices[i, 0] = int64(i)
+            new_sparse_values[i] = default_value_
+            empty_row_indicator[i] = int64(1)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[i, k] = int64(0)
+
+        return (new_sparse_indices, new_sparse_values, empty_row_indicator)
+
+    else:
+        for i in range(0, int64(sparse_indices[0, 0])):
+            new_sparse_indices[idx, 0] = int64(i)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[idx, k] = int64(0)
+
+            new_sparse_values[idx] = default_value_
+            empty_row_indicator[i] = int64(1)
+            idx += 1
+
+        for i in range(0, int64(sparse_indices.shape[0])):
+            index = int64(sparse_indices[i, 0])
+            if i == 0:
+                new_sparse_indices[idx, 0] = index
+                for k in range(1, int64(new_sparse_indices_shape[1])):
+                    new_sparse_indices[idx, k] = int64(sparse_indices[i, k])
+                new_sparse_values[idx] = int64(sparse_values[i])
+                empty_row_indicator[index] = int64(0)
+                idx += 1
+            else:
+                prev_index = int64(sparse_indices[i - 1, 0] + 1)

Review comment:
       If you structure this loop as handling indices from `sparse_indices[i,0]` to `sparse_indices[i+1,0]`, you can simplify the code by removing some of these index copying blocks.

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")
+    new_sparse_values = output_tensor(new_sparse_values_shape, "int64")
+    empty_row_indicator = output_tensor(empty_row_indicator_shape, "int64")
+    idx = 0
+
+    if int64(sparse_indices.shape[0]) == int64(0):
+        for i in range(0, int64(new_sparse_indices_shape[0])):
+            new_sparse_indices[i, 0] = int64(i)
+            new_sparse_values[i] = default_value_
+            empty_row_indicator[i] = int64(1)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[i, k] = int64(0)
+
+        return (new_sparse_indices, new_sparse_values, empty_row_indicator)
+
+    else:
+        for i in range(0, int64(sparse_indices[0, 0])):
+            new_sparse_indices[idx, 0] = int64(i)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[idx, k] = int64(0)
+
+            new_sparse_values[idx] = default_value_
+            empty_row_indicator[i] = int64(1)
+            idx += 1
+
+        for i in range(0, int64(sparse_indices.shape[0])):
+            index = int64(sparse_indices[i, 0])
+            if i == 0:
+                new_sparse_indices[idx, 0] = index

Review comment:
       nit: you're using two variables called index (`idx` and `index`). Could you make the names a little more specific.

##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1584,6 +1584,47 @@ RELAY_REGISTER_OP("repeat")
     .set_attr<FTVMCompute>("FTVMCompute", RepeatCompute)
     .set_attr<TOpPattern>("TOpPattern", kBroadcast);
 
+bool SparseFillEmptyRowsRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
+                            const TypeReporter& reporter) {
+  // types: [sparse_indices, sparse_values, dense_shape, default_value, result]
+  ICHECK_EQ(types.size(), 5);
+  std::vector<Type> fields;
+  auto sparse_indices = types[0].as<TensorTypeNode>();
+  auto ndims = sparse_indices->shape[1];
+  fields.push_back(TensorType(Array<PrimExpr>{Any(), ndims}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  fields.push_back(TensorType(Array<PrimExpr>{Any()}, tvm::DataType::Int(64)));
+  reporter->Assign(types[types.size() - 1], TupleType(Array<Type>(fields)));
+  return true;
+}
+
+Expr MakeSparseFillEmptyRows(Expr sparse_indices, Expr sparse_values, Expr dense_shape,
+                             Expr default_value) {
+  static const Op& op = Op::Get("sparse_fill_empty_rows");
+  return Call(op, {sparse_indices, sparse_values, dense_shape, default_value}, 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(4)
+    .add_argument("sparse_indices", "Tensor",
+                  "A 2-D int64 tensor of shape [N, ndims], which specifies the indices of the"
+                  "elements in the sparse tensor that contain nonzero values")
+    .add_argument("sparse_values", "Tensor",
+                  "A 1-D tensor[N] which supplies the values for each element in indices")
+    .add_argument("dense_shape", "Tensor",
+                  "A 1-D int64 tensor of shape [ndims], which specifies the dense_shape of the"
+                  "sparse tensor. Takes a list indicating the number of elements in each dimension")
+    .add_argument("default_value", "Tensor",

Review comment:
       Shouldn't this be a scalar?

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")
+    new_sparse_values = output_tensor(new_sparse_values_shape, "int64")
+    empty_row_indicator = output_tensor(empty_row_indicator_shape, "int64")
+    idx = 0
+
+    if int64(sparse_indices.shape[0]) == int64(0):
+        for i in range(0, int64(new_sparse_indices_shape[0])):
+            new_sparse_indices[i, 0] = int64(i)
+            new_sparse_values[i] = default_value_
+            empty_row_indicator[i] = int64(1)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[i, k] = int64(0)
+
+        return (new_sparse_indices, new_sparse_values, empty_row_indicator)
+
+    else:
+        for i in range(0, int64(sparse_indices[0, 0])):
+            new_sparse_indices[idx, 0] = int64(i)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[idx, k] = int64(0)
+
+            new_sparse_values[idx] = default_value_
+            empty_row_indicator[i] = int64(1)
+            idx += 1
+
+        for i in range(0, int64(sparse_indices.shape[0])):
+            index = int64(sparse_indices[i, 0])
+            if i == 0:
+                new_sparse_indices[idx, 0] = index
+                for k in range(1, int64(new_sparse_indices_shape[1])):
+                    new_sparse_indices[idx, k] = int64(sparse_indices[i, k])
+                new_sparse_values[idx] = int64(sparse_values[i])
+                empty_row_indicator[index] = int64(0)
+                idx += 1
+            else:
+                prev_index = int64(sparse_indices[i - 1, 0] + 1)
+                for j in range(prev_index, index):
+                    new_sparse_indices[idx, 0] = int64(j)
+                    for k in range(1, int64(new_sparse_indices_shape[1])):
+                        new_sparse_indices[idx, k] = int64(0)
+                    empty_row_indicator[prev_index] = int64(1)
+                    new_sparse_values[idx] = default_value_
+                    idx += 1
+
+                new_sparse_indices[idx, 0] = index
+                for k in range(1, int64(new_sparse_indices_shape[1])):
+                    new_sparse_indices[idx, k] = int64(sparse_indices[i, k])
+                new_sparse_values[idx] = int64(sparse_values[i])
+                empty_row_indicator[index] = int64(0)
+                idx += 1
+
+        for i in range(
+            int64(sparse_indices[sparse_indices.shape[0] - 1, 0] + 1), int64(dense_shape[0])

Review comment:
       Why is there a `+1` in this expression? If the sparse indices have size 1 and the dense shape is size 2, then we wouldn't add any elements.

##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1812,6 +1812,109 @@ def test_forward_sparse_dense_matmul():
     )
 
 
+#######################################################################
+# SparseFillEmptyRows
+# ------------
+
+
+def _test_sparse_fill_empty_rows(indices_np, values_np, dense_shape_np, default_value_int, use_dyn):
+    with tf.Graph().as_default():
+        if use_dyn:
+            indices = tf.placeholder(shape=(None, None), dtype=indices_np.dtype, name="indices")
+            values = tf.placeholder(shape=(None), dtype=values_np.dtype, name="values")
+            dense_shape = tf.placeholder(
+                shape=(None), dtype=dense_shape_np.dtype, name="dense_shape"
+            )
+        else:
+            indices = tf.placeholder(shape=indices_np.shape, dtype=indices_np.dtype, name="indices")
+            values = tf.placeholder(shape=values_np.shape, dtype=values_np.dtype, name="values")
+            dense_shape = tf.placeholder(
+                shape=dense_shape_np.shape, dtype=dense_shape_np.dtype, name="dense_shape"
+            )
+
+        default_value = tf.placeholder(shape=(), dtype=values_np.dtype, name="default_value")
+        sp_input = tf.sparse.SparseTensor(indices=indices, values=values, dense_shape=dense_shape)
+        _ = tf.sparse.fill_empty_rows(sp_input, default_value, name="sparse_fill_empty_rows")
+        compare_tf_with_tvm(
+            [indices_np, values_np, dense_shape_np, default_value_int],
+            [indices.name, values.name, dense_shape.name, default_value.name],
+            [
+                "sparse_fill_empty_rows/SparseFillEmptyRows:0",
+                "sparse_fill_empty_rows/SparseFillEmptyRows:1",
+                "sparse_fill_empty_rows/SparseFillEmptyRows:2",
+            ],
+            mode="vm",
+        )
+
+
+@pytest.mark.parametrize(
+    "sparse_indices_np, sparse_values_np, dense_shape_np, default_value_int",
+    [
+        (
+            np.array([[1, 1], [0, 3], [0, 1], [2, 0], [3, 1]], dtype=np.int64),

Review comment:
       This doesn't appear to be in the correct format. Shouldn't the first column be in sorted order?

##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1812,6 +1812,109 @@ def test_forward_sparse_dense_matmul():
     )
 
 
+#######################################################################
+# SparseFillEmptyRows
+# ------------
+
+
+def _test_sparse_fill_empty_rows(indices_np, values_np, dense_shape_np, default_value_int, use_dyn):
+    with tf.Graph().as_default():
+        if use_dyn:
+            indices = tf.placeholder(shape=(None, None), dtype=indices_np.dtype, name="indices")
+            values = tf.placeholder(shape=(None), dtype=values_np.dtype, name="values")
+            dense_shape = tf.placeholder(
+                shape=(None), dtype=dense_shape_np.dtype, name="dense_shape"
+            )
+        else:
+            indices = tf.placeholder(shape=indices_np.shape, dtype=indices_np.dtype, name="indices")
+            values = tf.placeholder(shape=values_np.shape, dtype=values_np.dtype, name="values")
+            dense_shape = tf.placeholder(
+                shape=dense_shape_np.shape, dtype=dense_shape_np.dtype, name="dense_shape"
+            )
+
+        default_value = tf.placeholder(shape=(), dtype=values_np.dtype, name="default_value")
+        sp_input = tf.sparse.SparseTensor(indices=indices, values=values, dense_shape=dense_shape)
+        _ = tf.sparse.fill_empty_rows(sp_input, default_value, name="sparse_fill_empty_rows")
+        compare_tf_with_tvm(
+            [indices_np, values_np, dense_shape_np, default_value_int],
+            [indices.name, values.name, dense_shape.name, default_value.name],
+            [
+                "sparse_fill_empty_rows/SparseFillEmptyRows:0",
+                "sparse_fill_empty_rows/SparseFillEmptyRows:1",
+                "sparse_fill_empty_rows/SparseFillEmptyRows:2",
+            ],
+            mode="vm",
+        )
+
+
+@pytest.mark.parametrize(
+    "sparse_indices_np, sparse_values_np, dense_shape_np, default_value_int",

Review comment:
       Could you add a test where the `sparse_indices_np` array is empty?

##########
File path: python/tvm/topi/sparse_fill_empty_rows.py
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHnew_sparse_indices WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=no-else-return, too-many-locals, too-many-arguments, too-many-branches
+# pylint: disable=undefined-variable, invalid-name
+"""SparseFillEmptyRows operator"""
+from ..te import hybrid
+
+
+@hybrid.script
+def _sparse_fill_empty_rows(
+    sparse_indices,
+    sparse_values,
+    dense_shape,
+    default_value,
+    new_sparse_indices_shape,
+    new_sparse_values_shape,
+    empty_row_indicator_shape,
+):
+    default_value_ = int64(default_value[0])
+    new_sparse_indices = output_tensor(new_sparse_indices_shape, "int64")
+    new_sparse_values = output_tensor(new_sparse_values_shape, "int64")
+    empty_row_indicator = output_tensor(empty_row_indicator_shape, "int64")
+    idx = 0
+
+    if int64(sparse_indices.shape[0]) == int64(0):
+        for i in range(0, int64(new_sparse_indices_shape[0])):
+            new_sparse_indices[i, 0] = int64(i)
+            new_sparse_values[i] = default_value_
+            empty_row_indicator[i] = int64(1)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[i, k] = int64(0)
+
+        return (new_sparse_indices, new_sparse_values, empty_row_indicator)
+
+    else:
+        for i in range(0, int64(sparse_indices[0, 0])):
+            new_sparse_indices[idx, 0] = int64(i)
+            for k in range(1, int64(new_sparse_indices_shape[1])):
+                new_sparse_indices[idx, k] = int64(0)
+
+            new_sparse_values[idx] = default_value_
+            empty_row_indicator[i] = int64(1)
+            idx += 1
+
+        for i in range(0, int64(sparse_indices.shape[0])):
+            index = int64(sparse_indices[i, 0])
+            if i == 0:
+                new_sparse_indices[idx, 0] = index
+                for k in range(1, int64(new_sparse_indices_shape[1])):
+                    new_sparse_indices[idx, k] = int64(sparse_indices[i, k])
+                new_sparse_values[idx] = int64(sparse_values[i])
+                empty_row_indicator[index] = int64(0)
+                idx += 1
+            else:
+                prev_index = int64(sparse_indices[i - 1, 0] + 1)

Review comment:
       Also, it appears that you require the indices to be in sorted order?




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