Hybrid search - fill and max normalise (Option 1b)#4763
Conversation
ellenmuller
changed the title
| // Raw cosine similarity, then mapped onto ES's Cosine `_score` scale of (1 + cos) / 2 | ||
| // so a locally-computed score is directly comparable to the knn scores ES returns. | ||
| def cosineSim(a: List[Double], b: List[Double]): Double = { | ||
| val dot = (a zip b).map { case (x, y) => x * y }.sum | ||
| val magA = math.sqrt(a.map(x => x * x).sum) | ||
| val magB = math.sqrt(b.map(y => y * y).sum) | ||
| if (magA == 0.0 || magB == 0.0) 0.0 else dot / (magA * magB) | ||
| } |
There was a problem hiding this comment.
If the vectors in ES are unit-normalised, the dot product is equivalent to cos here and you don't need the magnitude & division part - worth checking if they are, to avoid extra steps
There was a problem hiding this comment.
I think that query vectors will be, because we actually request them at 256 dims, but I think image vectors will not, because we've stored the truncated vector that we requested at 1536
| val rawBm25Score = bm25ScoreById.getOrElse(id, followUpBm25ScoreById.getOrElse(id, 0.0)) | ||
|
|
||
| // HNSW is approximate, so a locally-computed knn score can nudge above maxScore - clamp to 1. | ||
| val normKnn = if (knnMaxScore > 0.0) math.min(rawKnnScore / knnMaxScore, 1.0) else 0.0 |
There was a problem hiding this comment.
If we genuinely get a KNN score higher than the previous one, it doesn't really matter, so we don't need to do this capping here. If it's higher, it should get placed higher in the ranking
| .knn(knn) | ||
| .size(k) | ||
|
|
||
| val multiMatchQuery = createMultiMatchQuery(query, boost = Some(1.0)) |
There was a problem hiding this comment.
I don't think you need this boost?
| val multiMatchQuery = createMultiMatchQuery(query, boost = Some(1.0)) | |
| val multiMatchQuery = createMultiMatchQuery(query) |
| val normBm25 = if (bm25MaxScore > 0.0) rawBm25Score / bm25MaxScore else 0.0 | ||
|
|
||
| val combinedScore = vecWeight * normKnn + (1.0 - vecWeight) * normBm25 | ||
| (id, source, combinedScore) |
There was a problem hiding this comment.
could be a bit more readable to create a little case class for this instead of a tuple
| sourceById.get(id).map { source => | ||
| // knn score: ES knn score if present, else local cosine mapped to ES Cosine scale. | ||
| val rawKnnScore = knnScoreById.getOrElse(id, | ||
| bm25EmbeddingById.get(id).map(emb => (1.0 + cosineSim(queryEmbedding, emb)) / 2.0).getOrElse(0.0) |
There was a problem hiding this comment.
Everything else is beautifully readable but this line is a little dense. Could do with breaking out with variables/functions to make a bit clearer
There was a problem hiding this comment.
also you could use orElse to avoid the nested getOrElse e.g.
val rawKnnScore = knnScoreById.get(id)
.orElse(bm25EmbeddingById.get(id).map(localKnnScore))
.getOrElse(0.0)
joelochlann
changed the title
| bm25EmbeddingById.get(id).map(emb => (1.0 + cosineSim(queryEmbedding, emb)) / 2.0).getOrElse(0.0) | ||
| ) | ||
| // bm25 score: bm25 top-k score if present, else exact follow-up score, else 0. | ||
| val rawBm25Score = bm25ScoreById.getOrElse(id, followUpBm25ScoreById.getOrElse(id, 0.0)) |
There was a problem hiding this comment.
again, orElse could help you un-nest the getOrElse
3 participants
What does this change?
How should a reviewer test this change?
How can success be measured?
Who should look at this?
Tested? Documented?