Refactor cosine similarity and add cosine distance

[Shashi456]

Refer to this

cc: @rxwei.
Actually if we are removing the argument labels, i think we should do away with predicted and expected and have separate variable names.

[rxwei]

Thanks for the patch. However, what you did in this patch is not removing argument labels, but instead merely deleting comments. Here are the definitions of argument labels.

In Swift, functions names do not just include the base name, but also the argument labels. For the existing API, cosineSimilarity(predicted:expected:) is the function name, which includes argument labels predicted: and expected:. By suggesting changing the name to cosineSimilarity(_:_:), I included the suggested name change which is removing argument labels, which should manifest in code as:

public func cosineSimilarity<Scalar: TensorFlowFloatingPoint>(
    _ predicted: Tensor<Scalar>, _ expected: Tensor<Scalar>
) -> Tensor<Scalar>

Hope that clarifies things. Comments should not be changed.

[Shashi456]

@rxwei Ah, I'll fix this immediately. Thanks for taking time and explaining this to me.

[Shashi456]

@rxwei well the build passes locally. I cant build tests because i keep getting error since #234. Can you trigger a build?

[Shashi456]

@rxwei Why do some kokoro labels get blocked?

[eaplatanios]

This is not computing cosine similarity but rather the negative cosine similarity. I would suggest you remove the minus sign and also introduce a dual function called cosineDistance which is computed as 1 - cosineSimilarity.

[Shashi456]

Also @rxwei any idea on passing #31, one of the last losses to be added.

[Shashi456]

@rxwei could you retrigger a build?

[saeta]

Thanks for adding this @Shashi456 ! Looks almost there.

[saeta]

Could you please also add a test for cosine distance?

[Shashi456]

@saeta, actually cosine distance just returns 1 - cosineSimilarity, so in effect we would be testing the same function. To avoid redundancy I didn't add a test

[rxwei]

Even though the function is implemented as a trivial wrapper, it's good practice to assume the implementation can change in the future for whatever reasons and have tests be implementation-agnostic.

[rxwei]

Merging this to keep things rolling. It is fine to add a test for cosineDistance(_:_:) later.