[GitHub] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6889: [TOPI] sparse_dense Op sparse_data input added

[GitBox <gi...@apache.org>]

ANSHUMAN87 commented on a change in pull request #6889:
URL: https://github.com/apache/incubator-tvm/pull/6889#discussion_r524490253



##########
File path: src/relay/op/nn/sparse.cc
##########
@@ -85,10 +117,11 @@ RELAY_REGISTER_OP("nn.sparse_dense")
 )code" TVM_ADD_FILELINE)
     .set_attrs_type<SparseDenseAttrs>()
     .set_num_inputs(4)
-    .add_argument("data", "nD Tensor", "Input data.")
-    .add_argument("weight_data", "1D Tensor", "Weight data matrix.")
-    .add_argument("weight_indices", "1D Tensor", "Weight indices matrix.")
-    .add_argument("weight_indptr", "1D Tensor", "Weight indptr matrix.")
+    .add_argument("input_tensor1", "nD Tensor",

Review comment:
       I think we can not go back to their original names, as the tensor will be used for multiple purposes as given in the description string. I too am not impressed by the name "input_tensor1" :slightly_smiling_face: 
   Would appreciate if you can suggest some names.
   
   How about merger of both use case as below? 
   "input_tensor1"  => "data_dense_or_sparse"
   "input_tensor2"  => "weight_data_or_data_indices"
   "input_tensor3"  => "weight_indices_or_data_indptr"
   "input_tensor3"  => "weight_indptr_or_dense_weight"
   




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