Add Cosine Similarity #233
Conversation
Sources/TensorFlow/Loss.swift
Outdated
| public func cosineProximityLoss<Scalar: TensorFlowFloatingPoint>( | ||
| predicted: Tensor<Scalar>, expected: Tensor<Scalar> | ||
| ) -> Tensor<Scalar> { | ||
| return -(expected * predicted).sum() / (sqrt(expected.squared().sum()) * sqrt(predicted.squared().sum())) |
There was a problem hiding this comment.
Could you please break this line into two so that it fits within 100 columns? The second line should indent by 4 from the first line.
Sources/TensorFlow/Loss.swift
Outdated
| /// - predicted: Predicted outputs from a neural network. | ||
| /// - expected: Expected values, i.e. targets, that correspond to the correct output. | ||
| @differentiable(wrt: predicted) | ||
| public func cosineProximityLoss<Scalar: TensorFlowFloatingPoint>( |
There was a problem hiding this comment.
I think cosineDistance might be a better option.
There was a problem hiding this comment.
@rxwei @dan-zheng wdyt?
There was a problem hiding this comment.
I'd suggest calling this cosineSimilarity(_:_:) and make it differentiable w.r.t. both parameters.
Rationale:
- It is a standard measure of similarity and is the canonical name of this measure (it also has a Uncyclopedia page).
- It does not have to be a loss function, so it can be used in other contexts. Hence arguments labels differentiating between predicted and expected do not seem to be useful.
- Because of (2), we should make it differentiable with respect to both parameters.
There was a problem hiding this comment.
I’ve just noticed that the argument labels were not removed as suggested. Could you please start a PR to resolve it when you get a chance? Thanks!
There was a problem hiding this comment.
Thanks for taking this on! It'd be better if you fix it in a separate PR instead.
Reference #127