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

normalization in cosine similarity #33820

Open
ngc92 opened this issue Oct 29, 2019 · 2 comments
Assignees

Comments

@ngc92
Copy link
Contributor

@ngc92 ngc92 commented Oct 29, 2019

URL(s) with the issue:

https://www.tensorflow.org/api_docs/python/tf/keras/losses/cosine_similarity
https://www.tensorflow.org/api_docs/python/tf/keras/losses/CosineSimilarity

Description of issue (what needs changing):

The documentation for the cosine similarity does not state whether y_true and y_pred
are expected to be normalized vectors. The provided equation loss = -sum(y_true * y_pred)
suggests the need to be, but looking at the source, they are normalized as part of the computation.

y_true = nn.l2_normalize(y_true, axis=axis)
y_pred = nn.l2_normalize(y_pred, axis=axis)
return -math_ops.reduce_sum(y_true * y_pred, axis=axis)

As a special case, the doc does not state what happens in the case of either being zero.

(Also, isn't the above implementation suboptimal in terms of speed, as each element is divided by
the norm, instead of simply dividing the result once?)

@jvishnuvardhan

This comment has been minimized.

Copy link
Contributor

@jvishnuvardhan jvishnuvardhan commented Nov 1, 2019

@ngc92 Can you contribute through a PR to update the docs? Thanks!

@MarkDaoust

This comment has been minimized.

Copy link
Contributor

@MarkDaoust MarkDaoust commented Nov 3, 2019

Yes, can you send a PR?

The code is here:

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/keras/losses.py#L1097-L1141

If you make a PR please use doctest format, for example code so that it is testable (prefix code lines with >>>).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.