search

pytorch / glow

Compiler for Neural Network hardware accelerators
Apache License 2.0
3.21k stars 689 forks source link

Enhancing SparseToDense operator #4125

Open pdwaraka opened 4 years ago

pdwaraka commented 4 years ago

SparseToDense(S2D) in caffe2 differs in its functionality from Tensorflow and has below specifications. GLOW supports Caffe2 variant of S2D.

Caffe2 S2D only supports 1D sparse indices and only first output dimension is specified by "data_to_infer_dim". Here, accumulation is also performed for the case of same indices.

On the other hand, Tensorflow S2D operator supports 0D, 1D and 2D "sparse_indices" along with variable "default_value" and "output_shape". Does not perform accumulate operation because depending upon the "validate_indices" parameter, indices are expected to be unique or they are overwritten.

In such a case, should we write separate a operator to support tensorflow or enhance the existing S2D operator? Open for suggestions.

jfix71 commented 4 years ago

Hi @pdwaraka -- I think it makes sense to enhance the existing S2D operator. We could add a boolean member to the Node for whether to use accumulation. WDYT?

garg1himanshu commented 4 years ago

Hi @jfix71 Just adding a boolean to the existing node will not be enough as we can pass "default_value" and any "output_shape" to the TensorFlow operator. The existing Caffe2 operator takes default_value=0 and only 1st rank of output_shape can be passed. Given these differences, I think it will be better to have a different Node for TensorFlow S2D.

jfix71 commented 4 years ago

Hmm -- so as far as output_shape, we could simply add a ReshapeNode after the output of the SparseToDenseNode, which we can do cleanly inside Function::createSparseToDense(). For the default_value -- are you referring to SparseToDenseMask and not SparseToDense? Regardless, if we add the Cumulative flag to the Node/Instr, then we can just ignore default_value. Do you think this makes sense?