Repository navigation
Consolidate the cosineSimilarity parity helpers - #320
Merged
Merged
Conversation
DetectionDecoder and SegmentationPostprocessor each defined their own scalar `sigmoid(_:)`. The video decoder's branches on the sign so the exponent argument stays non-positive (overflow-safe); the segmenter's naive `1/(1+exp(-x))` underflows large-negative logits to exactly 0. Add one shared `sigmoid` in CoreAIShared/Runtime/VectorMath.swift (the sign-branched form) and route both subsystems and their tests through it, deleting the two duplicates. VectorMathTests characterizes both extremes. Intended behavior change: the segmenter path now returns a tiny positive value instead of exactly 0 for large-negative logits — negligible for any threshold > 0. WS4 PR2.
The image-segmenter, diffusion-runner, and speech-recognizer parity harnesses each defined their own `cosineSimilarity`. Add a shared one to CoreAIShared/Runtime/VectorMath.swift (Double accumulation) and route all three through it, deleting the copies. One zero-norm convention: empty, length-mismatched, and zero-norm inputs return 0. This changes the speech-recognizer harness's both-norms-zero case from 1 to 0; no parity fixture passes a zero-vector reference. The two `Float`-returning call sites wrap the result in `Float(...)` to keep their printed format. Adds cosineSimilarity cases to VectorMathTests.
Accumulate in Double, return Float. The image-segmenter and diffusion parity callers drop their `Float(...)` wrap; the speech parity harness casts to Double for its Double-based reporting.
…Similarity consolidation)
carinapeng
reviewed
Oct 9, 2026
carinapeng
approved these changes
Oct 9, 2026
carinapeng
left a comment
Contributor
There was a problem hiding this comment.
LGTM for a scoped change, please take a look at whether the exact match on all-zero tensors is consistent with the speech parity check
Two zero-norm (or empty) vectors are identical and now score 1; a single zero-norm vector still scores 0; a length mismatch scores 0. This restores the behavior the speech parity harness had before the helper was consolidated (both-zero -> 1), so every harness sharing cosineSimilarity agrees on the all-zero case. Addresses review feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The image-segmenter, diffusion-runner, and speech-recognizer parity harnesses each defined their own
cosineSimilarity. This adds a sharedcosineSimilaritytoCoreAIShared/Runtime/VectorMath.swift(Double accumulation) and routes all three through it.One zero-norm convention: empty, length-mismatched, and zero-norm inputs return 0. This changes the speech-recognizer harness's both-norms-zero case from 1 to 0; no parity fixture passes a zero-vector reference. The two
Float-returning call sites wrap the result inFloat(...)to keep their printed format.Adds
cosineSimilaritycases toVectorMathTests.Part of the shared-utility cleanup. Stacked on #317 (shared
sigmoid, same file): this PR's diff includes that change until #317 lands, then narrows to justcosineSimilarity.