Skip to content

Fix incorrect examples in distillation docs#6334

Merged
qgallouedec merged 2 commits into
mainfrom
fix-distillation-docs
Jul 9, 2026
Merged

Fix incorrect examples in distillation docs#6334
qgallouedec merged 2 commits into
mainfrom
fix-distillation-docs

Conversation

@sergiopaniego

@sergiopaniego sergiopaniego commented Jul 9, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Fixes several incorrect/misleading examples in the distillation documentation, all verified against the configs. Docs-only, no code changes.

  1. paper_index.md — GOLD snippet import fails. from trl.experimental import GOLDConfig raises ImportError (GOLDConfig isn't re-exported there). Changed to from trl.experimental.gold import GOLDConfig, matching every other snippet and examples/scripts/gold.py.

  2. paper_index.md — SDPO snippet silently disables feedback. The example set include_environment_feedback=False with the comment "Use dataset privileged_context when available" and lists privileged_context as an expected column. But the flag defaults to False, which gates off all environment feedback (feedback_used_fraction=0, distillation_loss stays 0) — the example looks wired up but trains on zero distillation signal. Set to True (matches sdpo_trainer.md).

  3. paper_index.md — broken MiniLLM cross-refs. [experimental.MiniLLMTrainer] / [experimental.MiniLLMConfig] don't resolve; the autodoc anchors are experimental.minillm.* (see minillm_trainer.md).

  4. gold_trainer.md — stray config flag. Removed model_revision from "configuration flags on GOLDConfig"; it is not a GOLDConfig field (only teacher_model_revision is) — it's a ModelConfig/CLI arg, so GOLDConfig(model_revision=...) would TypeError.

  5. minillm_trainer.mdgamma=Falsegamma=0.0. gamma is a float (default 0.0) and the surrounding text says γ=0; False is type-inconsistent.

  6. examples/scripts/sdft.py--teacher_update_rate 0.01 → 0.05. The script used 0.01 while the SDFTConfig default, the sdft_trainer.md example, and the trainer test all use 0.05. Aligned the script to 0.05.

Fixes # (issue): n/a

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue?
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

AI writing disclosure

  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.

Note

Low Risk
Documentation and example-only edits; no library or training logic changes.

Overview
Docs-only fixes so distillation examples match real TRL APIs and config fields.

paper_index.md: GOLD import is from trl.experimental.gold import GOLDConfig (the old trl.experimental import fails). MiniLLM doc links use experimental.minillm.* autodoc paths. The SDPO snippet sets include_environment_feedback=True so privileged_context is actually used (with False, distillation from feedback stays off).

gold_trainer.md: Drops model_revision from the GOLDConfig flag list—it belongs on model/CLI loading, not GOLDConfig.

minillm_trainer.md: Uses gamma=0.0 instead of gamma=False for the γ=0 on-policy case.

examples/scripts/sdft.py: Example CLI uses --teacher_update_rate 0.05 (was 0.01).

Reviewed by Cursor Bugbot for commit 7b6b28a. Bugbot is set up for automated code reviews on this repo. Configure here.

@bot-ci-comment

bot-ci-comment Bot commented Jul 9, 2026

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sergiopaniego sergiopaniego requested review from cmpatino and kashif July 9, 2026 09:11
The sdft.py example used --teacher_update_rate 0.01 while the SDFTConfig
default, the sdft_trainer.md example, and the trainer test all use 0.05.
Align the script to 0.05.

@cmpatino cmpatino left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Thank you for fixing these.

@qgallouedec qgallouedec merged commit bae8238 into main Jul 9, 2026
13 checks passed
@qgallouedec qgallouedec deleted the fix-distillation-docs branch July 9, 2026 18:15
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.

3 participants