Skip to content

Speed up educational BPE training - #600

Open
Marchematics wants to merge 2 commits into
openai:mainfrom
Marchematics:optimize-educational-bpe-training
Open

Speed up educational BPE training#600
Marchematics wants to merge 2 commits into
openai:mainfrom
Marchematics:optimize-educational-bpe-training

Conversation

@Marchematics

Copy link
Copy Markdown

Summary

Maintain BPE pair counts incrementally during training instead of rescanning the corpus for every merge.

This keeps the existing merge order and makes large vocabulary training practical.

Fixes #299

Tests

  • tests/test_educational.py
  • 1,000 randomized equivalence cases against the reference trainer
  • ruff check and python -m py_compile

Copilot AI lite review requested due to automatic review settings August 31, 2026 13:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Marchematics

Copy link
Copy Markdown
Author

Benchmark on 1,000 random 80-character words (500 merges): 4.79s -> 0.27s (17.8x).

The merge vocabulary matches the original trainer across 1,000 randomized cases.

@sylvesterkaczmarek

Copy link
Copy Markdown

Could we make the randomized equivalence validation mentioned in the PR reproducible in the committed test suite?

This replaces the simple corpus rescan with linked nodes, occurrence sets, and lazy heaps, where tie-breaking and overlapping merges are fairly easy to regress. The committed diff currently adds only two deterministic cases.

A bounded fixed-seed differential test against the previous/reference trainer on small corpora and vocabulary sizes would give this optimisation a durable correctness oracle in CI. Since the PR specifically addresses training performance, one reproducible before/after benchmark would also help quantify the improvement and memory trade-off.

Signed-off-by: Marchematics <Marchematics@163.com>
@Marchematics

Copy link
Copy Markdown
Author

Added fixed-seed differential coverage against the reference trainer for six small corpora/vocabulary pairs, plus scripts/benchmark_educational.py for reproducible timing and peak-memory measurements. The benchmark reports 4.321s -> 0.239s on the committed workload.

@sylvesterkaczmarek

Copy link
Copy Markdown

Added fixed-seed differential coverage against the reference trainer for six small corpora/vocabulary pairs, plus scripts/benchmark_educational.py for reproducible timing and peak-memory measurements. The benchmark reports 4.321s -> 0.239s on the committed workload.

Thanks, this addresses my concern. The committed differential coverage and reproducible benchmark make the optimisation much easier to validate and maintain.

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.

Tiktoken educational BPE trainer takes long time to train with vocab size 30k

4 participants