The tokenizer read every long word backwards (V-664)
encodeWord backtracks the Viterbi path from the end of the word and prepends each piece, which puts them back in reading order. A second reverse after that loop undid it. So "query: вода" tokenized to [0 12 1294 41 12489 2] where the reference tokenizer gives [0 41 1294 12 12489 2], and every multi-piece Russian word reached the model with its pieces in the wrong order. Measured on the recall fixture, same 27 cases either way: recall@1 70.4% -> 77.8% recall@3 85.2% -> 96.3% answered after gate 63.0% -> 66.7% false recall 0/5 -> 1/5 The classifier barely moves, 76.0% to 75.0% on the routing fixture, because seeds and queries were mangled the same way and cosine survived it. Recall is where it cost, because a stored passage and a live query are different lengths and break differently. The embedder id now names a tokenizer revision. Stored vectors were written under rev 1 and no longer sit in the same space as a query embedded now, and the model file's name never moved, so nothing would have triggered ReembedAll. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013ptwopxyo3Z2kwFckHkLvN
This commit is contained in:
@@ -1,10 +1,13 @@
|
||||
package router
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestEmbedderIDFromModelPath(t *testing.T) {
|
||||
got := modelIDFromPath("/opt/maven/models/embedder/multilingual-e5-small.onnx")
|
||||
if got != "multilingual-e5-small@384" {
|
||||
if got != "multilingual-e5-small@384/tok2" {
|
||||
t.Fatalf("modelIDFromPath = %q", got)
|
||||
}
|
||||
// A different model file must produce a different id, even at 384 dim.
|
||||
@@ -12,6 +15,13 @@ func TestEmbedderIDFromModelPath(t *testing.T) {
|
||||
if old == got {
|
||||
t.Fatal("two different models share one id")
|
||||
}
|
||||
// The tokenizer is half of what makes a vector, and it changes under a
|
||||
// model file whose name never moves (V-664). An id that ignored it would
|
||||
// leave stored passages in one space and every new query in another, with
|
||||
// nothing to trigger the re-embed.
|
||||
if !strings.Contains(got, "/tok") {
|
||||
t.Fatalf("id %q does not name the tokenizer revision", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmbedderIDIncludesDim(t *testing.T) {
|
||||
|
||||
@@ -66,12 +66,19 @@ func NewONNXEmbedder(modelPath, tokenizerPath, libPath string) (*onnxEmbedder, e
|
||||
func (e *onnxEmbedder) Dim() int { return embedDim }
|
||||
|
||||
// ID names the loaded model for the DB marker (Vikunja #378): the model file's
|
||||
// own name plus the dimension, so pointing the config at another model changes
|
||||
// the string on its own.
|
||||
// own name, the dimension, and the tokenizer revision, so pointing the config
|
||||
// at another model changes the string on its own.
|
||||
func (e *onnxEmbedder) ID() string { return e.id }
|
||||
|
||||
// tokenizerRev — bumped whenever the tokenizer changes what it emits for the
|
||||
// same text, because that changes every vector while the model file's name
|
||||
// stays put. Rev 2 is the fix for the reversed word pieces (V-664): stored
|
||||
// passages embedded under rev 1 no longer sit in the same space as a query
|
||||
// embedded now, and ReembedAll rewrites them because this string moved.
|
||||
const tokenizerRev = 2
|
||||
|
||||
// modelIDFromPath turns /opt/.../multilingual-e5-small.onnx into
|
||||
// "multilingual-e5-small@384".
|
||||
// "multilingual-e5-small@384/tok2".
|
||||
func modelIDFromPath(modelPath string) string {
|
||||
name := modelPath
|
||||
if i := strings.LastIndexAny(name, "/\\"); i >= 0 {
|
||||
@@ -81,7 +88,7 @@ func modelIDFromPath(modelPath string) string {
|
||||
if name == "" {
|
||||
name = "onnx"
|
||||
}
|
||||
return fmt.Sprintf("%s@%d", name, embedDim)
|
||||
return fmt.Sprintf("%s@%d/tok%d", name, embedDim, tokenizerRev)
|
||||
}
|
||||
|
||||
// Embed treats the text as a query. The classifier compares one short
|
||||
@@ -339,14 +346,17 @@ func (t *unigramTokenizer) encodeWord(word string) []int64 {
|
||||
}
|
||||
}
|
||||
|
||||
// Backtracking walks the word from its end, and prepending each piece puts
|
||||
// it back in reading order. There used to be a second reverse after this
|
||||
// loop, which undid it: every multi-piece word came out backwards, and
|
||||
// "query: вода" tokenized to [0 12 1294 41 12489 2] where the reference
|
||||
// tokenizer gives [0 41 1294 12 12489 2] (V-664). A transformer reads
|
||||
// position, so the pieces of a long Russian word were being read in the
|
||||
// wrong order on every turn.
|
||||
var result []int64
|
||||
for i := n; i > 0; i = prev[i] {
|
||||
result = append([]int64{bestID[i]}, result...)
|
||||
}
|
||||
// Reverse
|
||||
for l, r := 0, len(result)-1; l < r; l, r = l+1, r-1 {
|
||||
result[l], result[r] = result[r], result[l]
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user