docs(tutorial): merge the four training tutorials into one#840
docs(tutorial): merge the four training tutorials into one#840WindChimeRan wants to merge 9 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesDocumentation and training workflows
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
c1d0226 to
1cfbe4a
Compare
… and algorithm Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
1cfbe4a to
4df3744
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user_guide/algorithms/decision_guide.md`:
- Line 17: Update the verifier compatibility sentence to include DSpark
alongside Eagle-3, P-EAGLE, and DFlash in the unrestricted list, while
preserving the existing statement that MTP requires native MTP layers.
In `@docs/user_guide/getting_started.md`:
- Around line 40-44: Update the training summary and Supported Algorithms
content in the Getting Started guide to match the consolidated “Train a
Speculator” tutorial: describe all supported training modes and list or link
Eagle-3, P-EAGLE, DSpark, DFlash, and MTP consistently, without leaving
contradictory or incomplete algorithm information.
In `@docs/user_guide/tutorials/train.md`:
- Line 339: Document the DSpark --loss-fn argument in the CLI reference,
including its JSON format, accepted loss keys such as ce and tv, and expected
values, or link to authoritative documentation. Ensure the documented API and
examples match all usages in the tutorial, including the repeated {"ce": 0.1,
"tv": 0.9} example.
- Line 668: Update the checkpoint description near “Each checkpoint” to limit
the self-contained, deployment-ready claim to non-MTP speculator checkpoints.
State that MTP checkpoints require running stitch_mtp.py before serving, while
preserving the existing note about optimizer and learning-rate scheduler states
for resume training.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 334c94a3-9f9c-41a3-904e-c7e22ca6e85e
📒 Files selected for processing (26)
docs/.nav.ymldocs/cli/train.mddocs/user_guide/algorithms/decision_guide.mddocs/user_guide/algorithms/dflash.mddocs/user_guide/algorithms/dspark.mddocs/user_guide/algorithms/eagle3.mddocs/user_guide/algorithms/index.mddocs/user_guide/algorithms/mtp.mddocs/user_guide/algorithms/peagle.mddocs/user_guide/getting_started.mddocs/user_guide/tutorials/index.mddocs/user_guide/tutorials/response_regeneration.mddocs/user_guide/tutorials/train.mddocs/user_guide/tutorials/train_dflash_online.mddocs/user_guide/tutorials/train_eagle3_offline.mddocs/user_guide/tutorials/train_eagle3_online.mddocs/user_guide/tutorials/train_mtp_online.mddocs/user_guide/tutorials/train_peagle_offline.mdexamples/train/dflash_qwen3_8b_sharegpt_online_5k.shexamples/train/dspark_qwen3_0_6b_sharegpt_online.shexamples/train/eagle3_llama3_8b_ultrachat_offline_5k.shexamples/train/eagle3_qwen3_8b_sharegpt_online_5k.shexamples/train/mtp_qwen3_5_9b_gsm8k_online.shmkdocs.ymltests/e2e/regression/test_eagle3_online_acceptance.pytests/e2e/smoke/test_online_training.py
💤 Files with no reviewable changes (5)
- docs/user_guide/tutorials/train_mtp_online.md
- docs/user_guide/tutorials/train_dflash_online.md
- docs/user_guide/tutorials/train_eagle3_online.md
- docs/user_guide/tutorials/train_peagle_offline.md
- docs/user_guide/tutorials/train_eagle3_offline.md
|
|
||
| **MTP** finetunes the model's native multi-token prediction head on domain-specific data. Unlike the other algorithms, MTP does not train from scratch -- it starts from pre-existing MTP layers and is only available for models with native MTP support. | ||
|
|
||
| Eagle-3, P-EAGLE, and DFlash can be paired with any supported verifier model (including quantized variants) -- the draft architecture is independent of the verifier architecture. MTP requires a model with native MTP layers (e.g. Qwen3-Next, Qwen3.5). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include DSpark in the unrestricted verifier list.
This sentence omits DSpark even though docs/user_guide/algorithms/dspark.md and docs/user_guide/tutorials/train.md describe it as compatible with any supported verifier. Update the list to include DSpark while retaining the MTP exception.
As per path instructions, docs/**/*.md must be checked for clarity, accuracy, and completeness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user_guide/algorithms/decision_guide.md` at line 17, Update the verifier
compatibility sentence to include DSpark alongside Eagle-3, P-EAGLE, and DFlash
in the unrestricted list, while preserving the existing statement that MTP
requires native MTP layers.
Source: Path instructions
| - **Hybrid training** -- Hidden states are generated on-the-fly during the first epoch, cached, and reused after. | ||
|
|
||
| ### Tutorials | ||
|
|
||
| - [Train Eagle-3 Online](tutorials/train_eagle3_online.md) -- Recommended starting point | ||
| - [Train Eagle-3 Offline](tutorials/train_eagle3_offline.md) | ||
| - [Train DFlash Online](tutorials/train_dflash_online.md) -- Starting point for DFlash | ||
| - [Train a Speculator](tutorials/train.md) -- Recommended starting point. Covers Eagle-3, P-EAGLE, DFlash, DSpark, and MTP in all three modes |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the training summary consistent with the new tutorial.
The surrounding text still describes only online/offline training, and the Supported Algorithms section lists only Eagle-3 and DFlash while this entry advertises P-EAGLE, DSpark, and MTP. Update those statements or add the missing links so Getting Started does not contradict the consolidated tutorial.
As per path instructions, this Markdown must remain accurate and complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user_guide/getting_started.md` around lines 40 - 44, Update the training
summary and Supported Algorithms content in the Getting Started guide to match
the consolidated “Train a Speculator” tutorial: describe all supported training
modes and list or link Eagle-3, P-EAGLE, DSpark, DFlash, and MTP consistently,
without leaving contradictory or incomplete algorithm information.
Source: Path instructions
| --speculator-type dspark \ | ||
| --num-layers 5 \ | ||
| --lr 3e-4 \ | ||
| --loss-fn '{"ce": 0.1, "tv": 0.9}' \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the DSpark --loss-fn contract.
The DSpark commands introduce --loss-fn '{"ce": 0.1, "tv": 0.9}'—also repeated at Lines 461 and 586—but the CLI reference does not describe this argument or its accepted keys. Add it to docs/cli/train.md or link to authoritative DSpark loss documentation.
As per path instructions, this Markdown must be complete and its examples must match the documented API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user_guide/tutorials/train.md` at line 339, Document the DSpark
--loss-fn argument in the CLI reference, including its JSON format, accepted
loss keys such as ce and tv, and expected values, or link to authoritative
documentation. Ensure the documented API and examples match all usages in the
tutorial, including the repeated {"ce": 0.1, "tv": 0.9} example.
Source: Path instructions
| └── checkpoint_best -> 4/ # Symlink to lowest val loss checkpoint | ||
| ``` | ||
|
|
||
| Each checkpoint is a complete, self-contained speculator model ready for deployment in vLLM. The checkpoints also contain optimizer and learning rate scheduler states for resume training. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude MTP from the self-contained-checkpoint claim.
MTP checkpoints still require stitch_mtp.py before serving, as documented at Lines 364–370, 484–491, and 613–619. Saying every checkpoint is immediately deployable can lead users to serve the wrong artifact.
Proposed wording
-Each checkpoint is a complete, self-contained speculator model ready for deployment in vLLM.
+Non-MTP checkpoints are self-contained speculator models ready for deployment in vLLM.
+MTP checkpoints must be stitched into the verifier checkpoint before deployment.As per path instructions, this Markdown must remain accurate and complete.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Each checkpoint is a complete, self-contained speculator model ready for deployment in vLLM. The checkpoints also contain optimizer and learning rate scheduler states for resume training. | |
| Non-MTP checkpoints are self-contained speculator models ready for deployment in vLLM. MTP checkpoints must be stitched into the verifier checkpoint before deployment. The checkpoints also contain optimizer and learning rate scheduler states for resume training. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user_guide/tutorials/train.md` at line 668, Update the checkpoint
description near “Each checkpoint” to limit the self-contained, deployment-ready
claim to non-MTP speculator checkpoints. State that MTP checkpoints require
running stitch_mtp.py before serving, while preserving the existing note about
optimizer and learning-rate scheduler states for resume training.
Source: Path instructions
WindChimeRan
changed the title
speculatorsbot
left a comment
speculatorsbot
left a comment
There was a problem hiding this comment.
Well-structured docs refactor. Merging five tutorials into one tabbed page is a clear maintenance win, and the new DSpark algorithm page slots in cleanly. The tabbed layout with nested algorithm/mode selectors is a nice UX touch. Agree with @coderabbitai's suggestion to update the verifier-compatibility sentence in decision_guide.md to include DSpark, and the stale "Supported Algorithms" section in getting_started.md.
One item not yet raised: the five deleted tutorial files (train_eagle3_online.md, train_eagle3_offline.md, train_dflash_online.md, train_peagle_offline.md, train_mtp_online.md) will cause 404s for anyone who bookmarked those URLs or any external sites linking to them (search engine results, blog posts, the old ReadTheDocs versioned pages, etc.). Consider adding mkdocs-redirects (or the equivalent plugin already available in the Material theme) to map the old paths to the new consolidated page. Something like:
plugins:
- redirects:
redirect_maps:
user_guide/tutorials/train_eagle3_online.md: user_guide/tutorials/train.md
user_guide/tutorials/train_eagle3_offline.md: user_guide/tutorials/train.md
user_guide/tutorials/train_dflash_online.md: user_guide/tutorials/train.md
user_guide/tutorials/train_peagle_offline.md: user_guide/tutorials/train.md
user_guide/tutorials/train_mtp_online.md: user_guide/tutorials/train.mdNon-blocking, but worth doing to avoid link rot. Recommend approving once the inline items from CodeRabbit are addressed.
🤖 Generated with Claude Code using the /pr-review skill
| - **`--sample-from-anchor`** / **`--no-sample-from-anchor`** (bool, default: algorithm-specific) Whether to sample from the anchor position. `True`: sample from anchor and all mask positions (default for dspark, produces block_size tokens). `False`: anchor is bonus token (default for dflash, produces block_size-1 tokens). | ||
|
|
||
| - **`--max-anchors`** (int, default: `256`) Maximum anchor positions for DFlash training. | ||
| - **`--max-anchors`** (int, default: `3072`) Maximum anchor positions for DFlash training. |
There was a problem hiding this comment.
Should we really be using such a large max anchors by default? Commonly I see <1k in recent papers I've read, but haven't done large analysis. I guess this is just a docs update ultimately
There was a problem hiding this comment.
Agreed that we should look into this, but currently we've got the default as 3072 in train.py so this docs update makes sense for the time being.
There was a problem hiding this comment.
- This PR is just to let the doc match the code's default.
- I run some exp on ultrachat. 3072 is ok (it's indeed not ideal). Will run more exps & try some options in the near future ...
shanjiaz
left a comment
shanjiaz
left a comment
There was a problem hiding this comment.
Looks good in general! added some comments.
fynnsu
left a comment
fynnsu
left a comment
There was a problem hiding this comment.
Looks good! I think this approach is much cleaner.
Added a bunch of comments.
| - **`--sample-from-anchor`** / **`--no-sample-from-anchor`** (bool, default: algorithm-specific) Whether to sample from the anchor position. `True`: sample from anchor and all mask positions (default for dspark, produces block_size tokens). `False`: anchor is bonus token (default for dflash, produces block_size-1 tokens). | ||
|
|
||
| - **`--max-anchors`** (int, default: `256`) Maximum anchor positions for DFlash training. | ||
| - **`--max-anchors`** (int, default: `3072`) Maximum anchor positions for DFlash training. |
There was a problem hiding this comment.
Agreed that we should look into this, but currently we've got the default as 3072 in train.py so this docs update makes sense for the time being.
|
|
||
| Pick an algorithm and a training mode below; the rest of the walkthrough is the same for every combination. The examples use `Qwen/Qwen3-8B` as the target model -- except for MTP, which needs a verifier with native MTP layers and so uses `Qwen/Qwen3.5-9B`. The process is the same for other models. | ||
|
|
||
| ## Choose Your Setup |
There was a problem hiding this comment.
I think this "Choose Your Setup" section should also use the grouped tabs. That way the user can actually "choose" one at this step and when they scroll down that's what they'll see.
There was a problem hiding this comment.
We probably don't need the "what it changes in the train command" section either.
There was a problem hiding this comment.
We probably don't need the "what it changes in the train command" section either. agree. applied the change
There was a problem hiding this comment.
I think this "Choose Your Setup" section should also use the grouped tabs. That way the user can actually "choose" one at this step and when they scroll down that's what they'll see.
done
There was a problem hiding this comment.
Agreed that we should look into this, but currently we've got the default as 3072 in train.py
future PR
| # Speculators venv (for data prep and training) | ||
| uv venv speculators_venv | ||
| source speculators_venv/bin/activate | ||
| uv pip install "speculators>=0.6.0" |
There was a problem hiding this comment.
Maybe we should mention installing from main as an option here?
There was a problem hiding this comment.
- this PR's bug:
scripts/is not in the wheel. so the only valid way is installing from main scripts/is not a good UX. Will fix it in followup PRs ...- I think we should bump the version to 0.6.1 to include the DSpark bug fixing.
|
|
||
| ## Step 1: Prepare Your Data | ||
|
|
||
| **Recommended:** regenerate the dataset's responses with your target model first (see [Response Regeneration](response_regeneration.md)) and pass the resulting JSONL to `--data`. This on-policy data aligns the drafter with what the target actually generates. Using the dataset's original responses, as shown below, is a cheaper off-policy fallback that skips a full target-model pass over the data, at the cost of lower acceptance length. **For MTP this is not optional** -- it requires data generated by the target model itself. |
There was a problem hiding this comment.
Maybe we should make this it's own step? We can still call it recommended and just link to response regen, but that way the step isn't getting lost under the prepare data header.
| --seq-length 8192 | ||
| ``` | ||
|
|
||
| For MTP, point `--data` at a regenerated dataset. You can produce one with [Response Regeneration](response_regeneration.md), or download a pre-regenerated one: |
There was a problem hiding this comment.
Maybe we can generalize this comment so that it isn't just about MTP
| ```bash | ||
| # in speculators venv | ||
| hf download \ | ||
| inference-optimization/Qwen3.5-9B-responses gsm8k.jsonl \ | ||
| --repo-type dataset \ | ||
| --local-dir ./output/dataset | ||
|
|
||
| python scripts/prepare_data.py \ | ||
| --model Qwen/Qwen3.5-9B \ | ||
| --data ./output/dataset/gsm8k.jsonl \ | ||
| --output ./output \ | ||
| --max-samples 5000 \ | ||
| --seq-length 8192 | ||
| ``` |
There was a problem hiding this comment.
Maybe this should be hidden behind an MTP tab?
There was a problem hiding this comment.
Currently it's a MTP vs others. So there's no tab. Will delete non-regen path in train.md in the next PR.
Not this PR, because regen is not yet production-ready.
| ## Expected Results | ||
|
|
||
| These are sanity-check runs, not production numbers. With just 5K samples, model performance will be limited -- the point is to verify that the pipeline works and the model is learning. For production quality, train on significantly more data. | ||
|
|
There was a problem hiding this comment.
I don't like that we only have these results for DFlash and P-Eagle. Maybe we could generalize / remove the specific results and just:
- mention this is not a production run and real runs need a lot more data
- (Maybe) Give a ballpark estimate for position acceptance rates/length (e.g. for most methods we expect to see around 40% first token acceptance rate on similar data)
There was a problem hiding this comment.
I'd keep the DFlash and P-Eagle result for now (but I moved them out of the tab to be clearer), just wanna match the step 1 data.
They'll be all replaced by the new regen result in future PRs.
|
Oh and I think the speculators bot suggestion might make sense:
|
220464a to
bf704a4
Compare
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews
🔴 Require approval from approved reviewers listWaiting for any of
This rule is failing.All pull requests must have at least one approving review from a member of the approved reviewers list before merging.
|
Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
… are not in the PyPI package Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
…uisites Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
…idated page Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
…d explain the GPU split Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
5848cf7 to
25cf718
Compare
…o-algorithm tabs Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
25cf718 to
c99e5b0
Compare
…hat syncs to Steps 3 and 4 Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
…s in favor of vllm-project#849 Signed-off-by: Ranran Haoran Zhang <ranzhang@redhat.com>
|
speculatorsbot
left a comment
speculatorsbot
left a comment
There was a problem hiding this comment.
The new commits address the feedback from the first round well -- tabbed "Choose Your Setup" picker, torchrun as the default, redirects for deleted pages, repo-clone install, and accelerator-neutral prerequisites all look good.
One remaining issue: the last commit drops dspark.md (in favor of PR #849), but several files in this PR still contain markdown links that point to it. If this PR merges before #849, those links will 404 on the docs site. Even if #849 goes first, the dependency is implicit and easy to miss. Safest fix: remove the three broken dspark.md links from this PR and let #849 add them when it creates the page.
🤖 Generated with Claude Code using the /pr-review skill
| ## See Also | ||
|
|
||
| - [Train DFlash Tutorial](../tutorials/train_dflash_online.md) -- Step-by-step training guide | ||
| - [DSpark](dspark.md) -- Builds on DFlash with a sequential Markov head and a confidence head |
There was a problem hiding this comment.
Both links to dspark.md in this file (here and line 26) now point to a file that does not exist -- it was deleted in the last commit of this PR and does not exist on main. If this PR merges before #849, these produce 404s.
Consider reverting these two links to plain text (matching what main currently has), and letting #849 add them when it creates dspark.md:
| - [DSpark](dspark.md) -- Builds on DFlash with a sequential Markov head and a confidence head | |
| - DSpark -- Builds on DFlash with a sequential Markov head and a confidence head |
And at line 26, revert [DSpark](dspark.md) back to just DSpark.
|
|
||
| /// tab | DSpark | ||
|
|
||
| DFlash plus a Markov logit-bias head and a per-position confidence head. ([details](../algorithms/dspark.md)) |
There was a problem hiding this comment.
Same issue: ../algorithms/dspark.md does not exist on main or in this PR. This link will 404 if merged before #849.
| DFlash plus a Markov logit-bias head and a per-position confidence head. ([details](../algorithms/dspark.md)) | |
| DFlash plus a Markov logit-bias head and a per-position confidence head. |
The DSpark tab itself is fine (it documents real CLI flags), but the algorithm-page link should wait until the page exists.
5 participants
Easy review
ask claude code to pull the PR to local, then host it locally. Read the
speculators/user_guide/tutorials/train/page.This is a refactor only PR. it only actually touches the content of the
trainpages.Purpose
This is a doc refactoring PR.
Also, hybrid mode had no tutorial of its own (it was buried as an "Advanced" subsection of the Eagle-3 online page), DSpark had none at all, and MTP was only documented for online training even though the mode is orthogonal to it.
Tests
Docs only, no behavior change.
Followup PRs
cc: @guan404ming
Checklist
I have filled in: