Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does tf.math.equal supports tf.sparse.SparseTensor? #50189

Closed
HongtaoYang opened this issue Jun 10, 2021 · 8 comments
Closed

Does tf.math.equal supports tf.sparse.SparseTensor? #50189

HongtaoYang opened this issue Jun 10, 2021 · 8 comments
Assignees
Labels
comp:ops OPs related issues stat:awaiting response Status - Awaiting response from author type:bug Bug

Comments

@HongtaoYang
Copy link

HongtaoYang commented Jun 10, 2021

URL with the issue:

https://www.tensorflow.org/api_docs/python/tf/math/equal

Description of issue (what needs changing):

The doc says tf.math.equal supports sparse tensor as inputs, but it is actually not supported at the moment. See the example below. I haven't checked all math operations that claims to support sparse tensors, maybe there are other similar doc errors like this.

I'm using tf 2.6.0-dev20210601.

Usage example

import tensorflow as tf

a = tf.sparse.SparseTensor(
    indices=[[0, 0], [0, 1], [1, 2]],
    values=[1, 1, 1],
    dense_shape=[2, 3]
)

b = tf.sparse.SparseTensor(
    indices=[[0, 0], [0, 1], [1, 2]],
    values=[1, 1, 1],
    dense_shape=[2, 3]
)

tf.math.equal(a, b)  # raises ValueError

I think making equal operation supports sparse tensor is needed.

@tilakrayal
Copy link
Contributor

tilakrayal commented Jun 10, 2021

@Saduf2019
Was able to reproduce the issue with TF v2.4,v2.5 and TF-nightly. Please find the gist of it here. Thanks!

@tilakrayal tilakrayal added comp:ops OPs related issues stat:awaiting response Status - Awaiting response from author type:bug Bug and removed stat:awaiting response Status - Awaiting response from author labels Jun 10, 2021
@tilakrayal tilakrayal assigned Saduf2019 and unassigned tilakrayal Jun 10, 2021
@tilakrayal tilakrayal added the type:docs-bug Document issues label Jun 10, 2021
@Namangarg110
Copy link

Hey I'm new to contributing to open source, can I work on this issue?

@HongtaoYang
Copy link
Author

Thanks for working on the issue. I'm using this temporary hack as a workaround atm.

import tensorflow as tf
from tensorflow.sparse import SparseTensor


def sparse_tensor_not_equal(a: SparseTensor, b: SparseTensor) -> SparseTensor:
    diff = tf.math.abs(tf.sparse.add(tf.math.negative(a), b))
    diff = tf.cast(diff, bool)

    return diff

def sparse_tensor_equal(a: SparseTensor, b: SparseTensor) -> SparseTensor:
    diff = sparse_tensor_not_equal(a, b)
    is_equal = tf.sparse.map_values(tf.math.logical_not, diff)

    return is_equal


a = tf.sparse.SparseTensor(indices=[[0, 0], [0, 1], [1, 2]], values=[1, 1, 1], dense_shape=[2, 3])
b = tf.sparse.SparseTensor(indices=[[0, 1], [0, 2], [1, 1], [1, 2]], values=[1, 1, 1, 1], dense_shape=[2, 3])
print(sparse_tensor_equal(a, b))

@ymodak ymodak removed the type:docs-bug Document issues label Jun 14, 2021
@Saduf2019 Saduf2019 assigned ymodak and unassigned Saduf2019 Jun 15, 2021
@ymodak ymodak added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Jun 15, 2021
@edloper
Copy link
Contributor

edloper commented Jun 16, 2021

Looking at the code, it looks to me like tf.math.equal and tf.math.not_equal do not, and have never, supported sparse tensors. (And they only support IndexedSlices in so far as IndexedSlices can be converted to tensors, so it seems odd to call out support for IndexedSlices.) It looks like this erroneous documentation was added in e0e1efb.

It would certainly be possible to extend tf.math.equal to support sparse tensors. If this is done, we should be sure to document the semantics -- namely that tf.equal(x, y) iff tf.equal(tf.sparse.to_dense(x), tf.sparse.to_dense(y)) (so two sparse tensors are considered equal even if one has an explict nonzero where another has an implicit nonzero). Also, we should document that x and y are assumed to be in standard lexicographic order (and tf.sparse.reorder needs to be used if they are not).

As for other ops in math_ops.py that claim to support SparseTensors, I didn't see any similar documentation errors. In particular, the following claim to work and do work: tf.math.abs, tf.math.negative, tf.math.sign, tf.cast. Also, my_dense * my_sparse works (using the multiplication operator), though I don't see that fact documented anywhere. (In contrast, tf.math.multiply(my_dense, my_sparse) does not work).

@sushreebarsa
Copy link
Contributor

@HongtaoYang
Is this still an issue ?
Could you please refer to the comment above and let us know if it helps?
Thanks!

@sushreebarsa sushreebarsa assigned sushreebarsa and unassigned ymodak Apr 21, 2022
@sushreebarsa sushreebarsa added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Apr 21, 2022
@HongtaoYang
Copy link
Author

Thanks, the comment helps, thanks @edloper ! I'll close this issue.

@google-ml-butler
Copy link

Are you satisfied with the resolution of your issue?
Yes
No

copybara-service bot pushed a commit that referenced this issue Apr 27, 2022
…ly claimed to support SparseTensor inputs.

Github issue: #50189

PiperOrigin-RevId: 444931142
@edloper
Copy link
Contributor

edloper commented Apr 27, 2022

Fixed the documentation in 011e57d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:ops OPs related issues stat:awaiting response Status - Awaiting response from author type:bug Bug
Projects
None yet
Development

No branches or pull requests

7 participants