Skip to content

perf(train): select top-k in the trainers instead of ordering everything - #2379

Open
ArthurZucker wants to merge 3 commits into
feat/train_encode_splitfrom
perf/trainer-topk-selection
Open

perf(train): select top-k in the trainers instead of ordering everything#2379
ArthurZucker wants to merge 3 commits into
feat/train_encode_splitfrom
perf/trainer-topk-selection

Conversation

@ArthurZucker

Copy link
Copy Markdown
Collaborator

Three sites ordered a whole collection to consume a prefix of it. select_nth_unstable
answers 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. feed is outside the timer — only the ordering is measured.

n distinct words all counts equal 8 distinct counts
200,000 3.58× 3.84×
1,000,000 6.69× 8.17×

(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.rs collected every entry of word_counts — one per distinct word in the
corpus, commonly millions — ordered all of them, and took the first vocab_size
(default 30_000). The min_frequency filter also 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.

unigram.rs make_seed_sentence_pieces ordered one entry per distinct substring of
the 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.rs prune_sentence_pieces ordered every candidate to take pruned_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).
  • seed pieces: substrings are distinct; and two entries equal on both fields would push
    identical (string, score) pairs anyway.
  • prune: this one needed work. The sort was sort_bystable — and candidates is
    built by walking ids ascending, so equal losses already resolved by id. by_loss_then_id
    spells that out, which is what makes a selection equivalent. Its == bound also means
    an already-oversized new_pieces takes every candidate; that path keeps 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 list already at seed_size
still takes one entry.

Tests

Neither unigram branch had any coverage — the default seed_size of 1_000_000 means no
test-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:

  • WordLevel, over 9 vocabulary sizes × 5 minimum frequencies × with/without a special
    token. Verified it fails if the final ordering is dropped.
  • Seed pieces: a bounded seed_size must equal the front of an unbounded run. Exercises
    the new branch 7 times.
  • Candidate selection must agree with the stable sort it replaced.

Note

tk-train is excluded from the workspace, so make lint never covers it. It has 5
pre-existing clippy failures under --all-features, including an E0432 unresolved
import in bpe/parity_trainer.rs; none are in the files touched here, and I have left
them alone. Worth a separate look — the crate does not currently build under that feature
combination.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant