perf(train): select top-k in the trainers instead of ordering everything - #2379
Open
ArthurZucker wants to merge 3 commits into
Open
perf(train): select top-k in the trainers instead of ordering everything#2379ArthurZucker wants to merge 3 commits into
ArthurZucker wants to merge 3 commits into
Conversation
…of it `do_train` collected every entry of `word_counts`, ordered all of them, and then took `vocab_size` off the front. `word_counts` holds one entry per distinct word in the corpus -- commonly millions -- against a default `vocab_size` of 30_000, so this was O(n log n) work to answer a question that only depends on the first k. `select_nth_unstable_by` partitions at the boundary in O(n) average, and only the k that survive are ordered. The `min_frequency` filter moves ahead of the selection: its predicate is on the count, which is the ordering's primary key, so it commutes with the sort and shrinks n first. The result is unchanged, not merely equivalent-in-spirit. `cmp` breaks count ties on the word and the words are unique (they are map keys), so it is a total order -- the selected set and its order are exactly what ordering everything produced. A test asserts that against a reference implementation that still sorts everything, over deliberately tie-heavy input (200 words across 4 distinct counts, so the kept/dropped boundary always falls inside a run of equal counts) for 9 vocabulary sizes x 5 minimum frequencies x with and without a special token.
…ng all Two sites in the Unigram trainer ordered a whole collection to consume a prefix of it. `make_seed_sentence_pieces` ordered `substr_index` -- one entry per distinct substring of the flattened corpus that passed the filters -- and then walked it until `seed_sentencepieces` reached `seed_size` (default 1_000_000). On a real corpus that is far more entries than the bound, and the comparison is the expensive kind: the key is `(score, &[char])`, so every tie is settled by comparing character slices rather than integers. Partitioning at the bound first cuts both the number of comparisons and how many of them are slice compares. The substrings are distinct, so the key is a total order and the selected set is what the full sort produced; and two entries equal on both fields would push identical `(string, score)` pairs anyway. `prune_sentence_pieces` ordered every candidate to take `pruned_size` of them, once per EM iteration, so the saving repeats. This one needed care: the sort was `sort_by`, which is stable, and `candidates` is built by walking ids in ascending order -- so equal losses already resolved by id. `by_loss_then_id` spells that out, which makes the order total and is what lets a selection pick the same set. The `==` in the loop below means an already-oversized `new_pieces` never trips the break and takes every candidate; that path is left on the full sort rather than quietly changing what it selects. Both `wanted` computations mirror their loops exactly, including the seed loop's off-by-one: its length check runs after the push, so a `seed_sentencepieces` already at or past `seed_size` still takes one entry. Neither branch had any test coverage -- the default `seed_size` of 1_000_000 means no test-sized corpus reaches the selection at all, which an `eprintln!` in the branch confirmed. Two tests now cover them: the seed pieces for a bounded `seed_size` must equal the front of an unbounded run (checked over a deliberately repetitive corpus, so the boundary lands inside a run of tied scores), and the candidate selection must agree with the stable sort it replaced on tie-heavy input. The first exercises the new branch seven times.
The measurement behind the two commits before this one, so the numbers in
their messages can be re-run rather than taken on trust.
Sweeps the distinct-word count with the vocabulary held at the default
30_000, because the saving is a function of n/k. `feed` is outside the timer
-- it builds the count map, which those commits do not touch -- so only the
ordering is measured. `spread` varies how many distinct counts the words are
spread over, which controls how often the comparator falls through to its
word tie-break.
cargo run --release --example topk -- 200000,1000000 3 8
Interleaved base/new, minimum over passes, on an M-series laptop:
n distinct all counts equal 8 distinct counts
200_000 3.58x 3.84x
1_000_000 6.69x 8.17x
Tie density turns out not to matter much, which was not the guess: the cost
is the number of comparisons, not the price of each one.
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.
Three sites ordered a whole collection to consume a prefix of it.
select_nth_unstableanswers that in O(n). 6.7–8.2× on WordLevel training with a million distinct words.
Output byte-identical.
Measured
cargo run --release --example topk -- 200000,1000000 3 8, interleaved base/new,minimum over passes.
feedis outside the timer — only the ordering is measured.(1M distinct: 131 ms → 20 ms.) The speedup grows with n/k, which is what an
O(n log n) → O(n) change should do. Tie density barely matters — the cost is the number
of comparisons, not the price of each one.
The three sites
wordlevel.rscollected every entry ofword_counts— one per distinct word in thecorpus, commonly millions — ordered all of them, and took the first
vocab_size(default 30_000). The
min_frequencyfilter also moves ahead of the selection: itspredicate is on the count, which is the ordering's primary key, so it commutes with the
sort and shrinks n first.
unigram.rsmake_seed_sentence_piecesordered one entry per distinct substring ofthe flattened corpus to consume
seed_size(default 1_000_000) of them. The key is(score, &[char]), so ties compare character slices rather than integers.unigram.rsprune_sentence_piecesordered every candidate to takepruned_size,once per EM iteration, so the saving repeats.
Why the output is unchanged
Each site's comparator is a total order, so "select then order k" picks the same set in
the same order as "order everything":
wordlevel: counts tie-break on the word, and words are unique (map keys).identical
(string, score)pairs anyway.sort_by— stable — andcandidatesisbuilt by walking ids ascending, so equal losses already resolved by id.
by_loss_then_idspells that out, which is what makes a selection equivalent. Its
==bound also meansan already-oversized
new_piecestakes every candidate; that path keeps the full sortrather than quietly changing what it selects.
Both
wantedcomputations mirror their loops exactly, including the seed loop'soff-by-one: its length check runs after the push, so a list already at
seed_sizestill takes one entry.
Tests
Neither unigram branch had any coverage — the default
seed_sizeof 1_000_000 means notest-sized corpus reaches the selection, which an
eprintln!in the branch confirmed(zero hits across the whole suite). Three tests added, each checked against a reference
that still sorts everything, on deliberately tie-heavy input so the kept/dropped boundary
lands inside a run of ties:
token. Verified it fails if the final ordering is dropped.
seed_sizemust equal the front of an unbounded run. Exercisesthe new branch 7 times.
Note
tk-trainisexcluded from the workspace, somake lintnever covers it. It has 5pre-existing clippy failures under
--all-features, including anE0432unresolvedimport in
bpe/parity_trainer.rs; none are in the files touched here, and I have leftthem alone. Worth a separate look — the crate does not currently build under that feature
combination.