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/10/14 18:26:56 UTC
[GitHub] [incubator-tvm] ANSHUMAN87 opened a new pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
ANSHUMAN87 opened a new pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685
SparseTensorDenseMatMul support added for Tensorflow frontend.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r516214740
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
I understand your confusion here.
I am refering the [link](https://docs.scipy.org/doc/scipy/reference/generated/scipy.sparse.bsr_matrix.html#scipy.sparse.bsr_matrix) .
As you can see the 1-D data is not only fed to bsr_matrix creation, also the coordinates (rows, cols) are provided alomg with shape. With this constructor , it internally forms indptrs & indices. But in our case, we have set of input as data(R x C) , indices & indptrs already, so we are utilizing different constructor.
I hope it helps to resolve your query here. 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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507870118
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
CSR format is just BSR format with block size one. You should be able to use the same indices and indptr as you do with CSR. What was there error you were getting?
----------------------------------------------------------------
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] [incubator-tvm] jroesch commented on pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#issuecomment-709673851
cc @tkonolige
----------------------------------------------------------------
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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r516151334
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -903,6 +903,46 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from scipy
+ from scipy.sparse import csr_matrix
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create scipy sparse Tensor(CSR)
+ weight_sp = csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
+
+ # If both are true means First input was dense and second was sparse
+ # TODO: Support other adjoint option too
+ if attr.get("adjoint_a") and attr.get("adjoint_b"):
+ ret = _op.transpose(ret)
+ else:
+ raise tvm.error.OpAttributeUnImplemented("Adjoint option is not supported yet.")
Review comment:
Could you make this error message a little more specific. Something like "Only tf.sparse.sparse_dense_matmul with adjoint_a=True and adjoint_b=True is supported, but adjoint_a=X and adjoint_b=Y was supplied."
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
Still confused on why this is necessary. The scipy docs show an example where a 1D data array is passed in to `bsr_matrix`. I guess it really doesn't matter though.
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
It seems like the different constructors have different behavior. Interesting.
Thanks for clearing that up.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#issuecomment-708582002
@FrozenGene , @siju-samuel , @kevinthesun : please help review. 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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507470035
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
+ """ One iteration of sparse_dense_matmul """
+
+ # TODO: Support adjoint options too
+ for adjoint_a in [False]:
+ for adjoint_b in [False]:
+ with tf.Graph().as_default():
+ A_sp = tf.sparse.SparseTensor(
+ indices=[[0, 0], [1, 2]], values=[4.0, 8.0], dense_shape=A_shape
+ )
+ B = tf.placeholder(shape=B_shape, dtype=dtype, name="B")
+
+ if flip:
+ result = tf.sparse.sparse_dense_matmul(
+ B, A_sp, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+ else:
+ result = tf.sparse.sparse_dense_matmul(
+ A_sp, B, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+
+ B_np = np.random.uniform(high=5.0, size=B_shape).astype(dtype)
+
+ # TODO: There is an issue in cuda scheduling for csr, work in progress
+ compare_tf_with_tvm([B_np], [B.name], result.name, no_gpu=True)
+
+
+def test_forward_sparse_dense_matmul():
+ """ sparse_dense_matmul op test"""
+ ###################################################################
+ #
+ # In order to create a SparseTensor, it requires 3 input as below:
+ # SparseTensor(indices=[[0, 0], [1, 2]], values=[1, 2], dense_shape=[3, 4])
+ #
+ # Above Sparse can be represented in Dense as below :
+ # [[1, 0, 0, 0]
+ # [0, 0, 2, 0]
+ # [0, 0, 0, 0]]
+ #
+ # ------------------------------------------------------------------
+
+ # TODO: False case for flip need to be supported
Review comment:
As we need to implement new Op to support better, maybe we take it up in next PR!
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r508589066
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
Review comment:
I totally agree with your comment, but as this PR is just an initial PR, i feel lets keep the code as it is, it provides better readability in terms of steps involved, later on, once all features are merged, we can work together to optimize it :slightly_smiling_face:
Please let me know, in case you think otherwise!
----------------------------------------------------------------
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] [incubator-tvm] siju-samuel commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r505991645
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
Review comment:
use `from scipy.sparse import csr_matrix`
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r508247633
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
On paper, I totally agree with you. But when it comes to implementation, it differs always. For example, your data representation, csr.data & bsr.data has different way of representation. As i could remember, it was an error while deducing R & C parameter from csr.data, and that is quite expected, as the block size is not provided, it will deduce it from the 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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507871629
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
Review comment:
If you swap rows and columns here you can avoid the sparse transpose below. This probably isn't much of a performance hit except for large matrices.
----------------------------------------------------------------
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] [incubator-tvm] siju-samuel merged pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
siju-samuel merged pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507173799
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
Review comment:
Thanks @tkonolige ! It was missed while partial merge.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507173007
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
+ """ One iteration of sparse_dense_matmul """
+
+ # TODO: Support adjoint options too
+ for adjoint_a in [False]:
+ for adjoint_b in [False]:
+ with tf.Graph().as_default():
+ A_sp = tf.sparse.SparseTensor(
+ indices=[[0, 0], [1, 2]], values=[4.0, 8.0], dense_shape=A_shape
+ )
+ B = tf.placeholder(shape=B_shape, dtype=dtype, name="B")
+
+ if flip:
+ result = tf.sparse.sparse_dense_matmul(
+ B, A_sp, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+ else:
+ result = tf.sparse.sparse_dense_matmul(
+ A_sp, B, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+
+ B_np = np.random.uniform(high=5.0, size=B_shape).astype(dtype)
+
+ # TODO: There is an issue in cuda scheduling for csr, work in progress
+ compare_tf_with_tvm([B_np], [B.name], result.name, no_gpu=True)
Review comment:
Sure i am working on it. 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] [incubator-tvm] siju-samuel commented on pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
siju-samuel commented on pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#issuecomment-721505427
Thanks @ANSHUMAN87 @tkonolige This PR is merged.
@ANSHUMAN87 please follow up the TODOs..
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#issuecomment-709001815
Looks like Tensorflow version in CI is older, may be we need to bump it!!!
----------------------------------------------------------------
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] [incubator-tvm] siju-samuel commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
siju-samuel commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r505991920
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
Review comment:
Numpy > Scipy
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
+
+ # If both are true means First input was dense and second was sparse
+ # TODO: Support other adjoint option too
+ if attr.get("adjoint_a") and attr.get("adjoint_b"):
Review comment:
return not supported error for other adjoint options
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
+ """ One iteration of sparse_dense_matmul """
+
+ # TODO: Support adjoint options too
+ for adjoint_a in [False]:
+ for adjoint_b in [False]:
+ with tf.Graph().as_default():
+ A_sp = tf.sparse.SparseTensor(
+ indices=[[0, 0], [1, 2]], values=[4.0, 8.0], dense_shape=A_shape
+ )
+ B = tf.placeholder(shape=B_shape, dtype=dtype, name="B")
+
+ if flip:
+ result = tf.sparse.sparse_dense_matmul(
+ B, A_sp, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+ else:
+ result = tf.sparse.sparse_dense_matmul(
+ A_sp, B, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+
+ B_np = np.random.uniform(high=5.0, size=B_shape).astype(dtype)
+
+ # TODO: There is an issue in cuda scheduling for csr, work in progress
+ compare_tf_with_tvm([B_np], [B.name], result.name, no_gpu=True)
Review comment:
Need a followup pr to solve the cuda scheduling issue for csr
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
+ """ One iteration of sparse_dense_matmul """
+
+ # TODO: Support adjoint options too
+ for adjoint_a in [False]:
+ for adjoint_b in [False]:
+ with tf.Graph().as_default():
+ A_sp = tf.sparse.SparseTensor(
+ indices=[[0, 0], [1, 2]], values=[4.0, 8.0], dense_shape=A_shape
+ )
+ B = tf.placeholder(shape=B_shape, dtype=dtype, name="B")
+
+ if flip:
+ result = tf.sparse.sparse_dense_matmul(
+ B, A_sp, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+ else:
+ result = tf.sparse.sparse_dense_matmul(
+ A_sp, B, adjoint_a=adjoint_a, adjoint_b=adjoint_b
+ )
+
+ B_np = np.random.uniform(high=5.0, size=B_shape).astype(dtype)
+
+ # TODO: There is an issue in cuda scheduling for csr, work in progress
+ compare_tf_with_tvm([B_np], [B.name], result.name, no_gpu=True)
+
+
+def test_forward_sparse_dense_matmul():
+ """ sparse_dense_matmul op test"""
+ ###################################################################
+ #
+ # In order to create a SparseTensor, it requires 3 input as below:
+ # SparseTensor(indices=[[0, 0], [1, 2]], values=[1, 2], dense_shape=[3, 4])
+ #
+ # Above Sparse can be represented in Dense as below :
+ # [[1, 0, 0, 0]
+ # [0, 0, 2, 0]
+ # [0, 0, 0, 0]]
+ #
+ # ------------------------------------------------------------------
+
+ # TODO: False case for flip need to be supported
Review comment:
I suggest this pr can include flip false case as well
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507174110
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
Review comment:
Sorry for late response!
Actually Tensorflow support sparse and dense combination for both A & B, utilizing adjoint option.
Currently, the normal order A sparse & B dense is yet to be supported(i am working on it currently), even mentioned as TODO as well. Please let me know in case i have not clarified your query. 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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507877890
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
Review comment:
In the code you have here, it looks like you are computing B (A^T)^T. (A^T is from the transpose you've applied, and B (A^T)^T is from the sparse dense). You should be computing A B because this is what tensorflow computes. I think what you want to do here is 1. transpose B, 2. sparse_dense with the transposed B and _untransposed_ A, 3. transpose the result. This is if `adjoint_a` and `adjoint_b` are false.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#issuecomment-720382734
----------------------------------------------------------------
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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r506601559
##########
File path: tests/python/frontend/tensorflow/test_forward.py
##########
@@ -1750,6 +1750,64 @@ def test_forward_batch_matmul():
_test_batch_matmul((2, 3, 4, 2, 3, 4, 5, 6), (2, 3, 4, 2, 3, 4, 5, 6), "float32", False, True)
+#######################################################################
+# SparseTensorDenseMatMul
+# ----------------------------------
+
+
+def _test_sparse_dense_matmul(indices, values, A_shape, B_shape, dtype, flip=False):
Review comment:
You pass in indices and values here, but then you don't use them? Or am I missing something?
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
Review comment:
Reading the Tensorflow docs (https://www.tensorflow.org/api_docs/python/tf/sparse/sparse_dense_matmul), it looks like Tensorflow is computing A*B where A is sparse. Our sparse_dense computes B*A^T. You've done the transposition, but the order seems wrong.
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
Why this change? If you pass a 1D array to `bsr_matrix` it work just fine.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r507174343
##########
File path: python/tvm/topi/cuda/sparse.py
##########
@@ -367,9 +367,14 @@ def _alter_sparse_dense_layout(_attrs, inputs, _tinfos, _out_type):
and isinstance(inputs[2], relay.Constant)
and isinstance(inputs[3], relay.Constant)
):
- sparse_matrix = sp.bsr_matrix(
- (inputs[1].data.asnumpy(), inputs[2].data.asnumpy(), inputs[3].data.asnumpy())
- )
+ if len(inputs[1].data.asnumpy().shape) == 1:
Review comment:
Actually the indices and ind pointers are in CSR format, so BSR_Matrix can not digest it in scipy. I got an error here.
----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r508585845
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
Review comment:
I think there is slight misunderstanding here. Please allow me to put a better picture here, then we can discuss further.
Tensorflow Sparse Dense Matmul:
`tf.sparse.sparse_dense_matmul(A, B, adjoint_a, adjoint_b) , where both A and B can be Sparse or Dense.`
So based on combination of input types, the adjoint option will be set. But internal matmul is always in order of A(Sparse) * B(Dense). This is implemented in Tensorflow in order to reuse the existing internal Op implemented.
For example:
When i feed D (Dense) and S (Sparse) as in our case(TVM currently support only this order),
Tensorflow pseudo-code works as follows:
(S^T * D^T )^T => D * S
As a side note: the last transpose i added in frontend, is to counter the transpose Op added in the Tensorflow graph subsequently.
I know its little confusing, but it would make sense once we workout an example based on above. Please let me know in case any further query. 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] [incubator-tvm] tkonolige commented on a change in pull request #6685: [Relay][Frontend] SparseTensorDenseMatMul support for Tensorflow
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #6685:
URL: https://github.com/apache/incubator-tvm/pull/6685#discussion_r508675167
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -890,6 +890,44 @@ def _impl(inputs, attr, params, mod):
return _impl
+def _sparse_tensor_dense_matmul():
+ # Sparse utility from Numpy
+ from scipy import sparse
+
+ def _impl(inputs, attr, params, mod):
+ assert len(inputs) == 4, "There should be 4 input tensors"
+
+ indices_tensor = _infer_value(inputs[0], params, mod).asnumpy()
+ values_tensor = _infer_value(inputs[1], params, mod).asnumpy()
+ dense_shape_tensor = _infer_value(inputs[2], params, mod).asnumpy()
+
+ data = inputs[3]
+
+ rows = [x[0] for x in indices_tensor]
+ cols = [x[1] for x in indices_tensor]
+
+ # Create Numpy sparse Tensor(CSR)
+ weight_sp = sparse.csr_matrix(
+ (values_tensor, (rows, cols)), shape=tuple(dense_shape_tensor.tolist())
+ )
+ weight_sp = sparse.csr_matrix(weight_sp.transpose())
+
+ weight_data = _expr.const(weight_sp.data, weight_sp.data.dtype)
+ weight_indptrs = _expr.const(weight_sp.indptr, weight_sp.indptr.dtype)
+ weight_indices = _expr.const(weight_sp.indices, weight_sp.indices.dtype)
+
+ ret = _op.nn.sparse_dense(data, [weight_data, weight_indices, weight_indptrs])
Review comment:
I was missing the part where `adjoint_a` and `adjoint_b` are required. Looks good to me.
----------------------------------------------------------------
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