docs: fix stale paths, flags, env vars and metric names - #2651
Conversation
Audited docs/ against main. Fixes only the claims that are wrong, not coverage gaps. Commands that fail as written: - glm4-5 / glm4-7-flash conversion used scripts/model_args.py, which does not exist; the helper is miles/utils/external_utils/model_args_utils.py - cli-reference listed on_policy_distillation as an --advantage-estimator choice; it is not in the choices list, OPD is --use-opd on top of any estimator - ci/02-docker-build and developer/versions used --variant rocm-mi300 / rocm-mi350; build.py and docker-build.yml define rocm700-mi30x, rocm700-mi35x and rocm720-mi35x, and rocm700-mi35x was missing entirely - inkling documented --optimizer-state-nvme-dir / -chunk-mb, which do not exist; the launcher passes --offload-train-target disk (colocated) or --stream-optimizer-state-to-disk + --offload-train-disk-chunk-mb (fully-async), and picks grad reduce dtype the same way - --sglang-enable-ep-moe / -deepep / -deepep-moe / -overlap-schedule are gone from the pinned sglang ServerArgs; recipes use --sglang-ep-size, --sglang-moe-a2a-backend, --sglang-moe-runner-backend, --sglang-deepep-mode - the rollout-correction example presented mis.yaml keys as CLI flags and documented a --use-rollout-correction flag that does not exist; the interface is --use-tis plus --custom-config-path / --custom-tis-function-path Symbols and paths that do not exist: - miles/ray/rollout.py is a package now; point at rollout_manager.py, rollout_server.py, server_group.py and train_data_conversion.py - deepseek-v4-flash claimed the launcher sets MILES_HACK_TRAIN_TORCH_DETERMINISTIC=1; list the five env vars it actually sets, plus the three --train-deterministic adds - deepseek-v4-flash described with_transformers_patch() in miles/utils/transformers_patch.py; both are gone - eval/skipped_unhealthy and eval/skipped_pin_violation have no emitter - the mis_* / mismatch_* metric names are emitted as tis_*, rs_* and unprefixed names - monitoring pointed at miles/utils/wandb_utils.py, now under tracking_utils/ Wrong values and premises: - --update-weight-buffer-size defaults to 512 MB, not 1 GB - argument-groups opened with "launch scripts are bash arrays"; scripts/ holds 34 Python launchers and no shell scripts, and misc_args was missing from the group inventory - architecture sent new reward types to sglang_rollout.py; the rm_type dispatch is in miles/rollout/rm_hub/ - --rm-type enumerations omitted gemma_math (used by the gemma-4 recipe), deterministic_random and the boxed_ prefix - the canonical CI label list omitted eval, fully-async, miles-plugin and amd, the last of which ci/00-stage.md already refers to
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review |
There was a problem hiding this comment.
LGTM — docs-only PR, and the four inline nits found are self-contained documentation inconsistencies (a stale claim in a file this PR didn't touch, and two spots where the PR's own rewritten metric/env-var tables don't fully match the wiring they describe). None affect runtime behavior or code correctness.
Extended reasoning...
Overview
This PR only touches Markdown files under docs/ and two examples/**/README.md files (the second pair are mirrored into generated docs via a checked pre-commit hook). It corrects stale paths, removed flags, wrong defaults, and renamed modules across ~27 files, replacing them with values cross-checked against the current source (arguments.py, docker/build.py, scripts/run_*.py, mis.py, etc.). No application code, build scripts, or CI configuration is modified.
Security risks
None. This is a documentation-only change with no executable code paths, no secrets, and no changes to auth, CI permissions, or build artifacts.
Level of scrutiny
Low-to-moderate is appropriate: the change is large in file count but each edit is a mechanical fact correction (path rename, removed flag, corrected default, renamed metric), not a design decision. The bug-hunting pass found four nits, all of the same class the PR itself is trying to eliminate — a stale cross-reference in an untouched file, an opt-in-vs-default framing error, a metrics table missing a prefix its own recommended wiring adds, and two doc rows describing metrics with a stale premise. These are useful corrections but don't change any behavior and don't block merging a docs PR whose overall content is accurate and net-positive.
Other factors
The author's PR description shows a careful methodology (grep-based audits, verification against pre-commit run --all-files and sync_example_docs.py --check), and explicitly scopes out coverage gaps as follow-up work rather than conflating them with this fix pass. Given the docs-only nature and the nit-level severity of the findings, human review is optional rather than required here.
| <a id="misc-args"></a> | ||
| ## `misc_args` - GPU layout and everything else | ||
|
|
||
| `misc_args` carries what the other groups do not: how many GPUs the actor gets, whether | ||
| it shares them with the rollout engines, the Megatron knobs a recipe pins once and never | ||
| tunes, and the optional dashboard. | ||
|
|
||
| Common entries: | ||
|
|
||
| | Concern | Flags | | ||
| |---|---| | ||
| | GPU layout | `--actor-num-nodes`, `--actor-num-gpus-per-node`, `--num-gpus-per-node` | | ||
| | Colocation | `--colocate` | | ||
| | Numerics pinned by the recipe | `--attention-dropout 0.0`, `--hidden-dropout 0.0`, `--attention-softmax-in-fp32`, `--accumulate-allreduce-grads-in-fp32` | | ||
| | Attention kernel | `--attention-backend` | | ||
| | Observability | `--use-miles-dashboard`, `--dump-details` | | ||
|
|
||
| Under `--colocate` the actor and the engines share the same GPUs and take turns, so | ||
| `--rollout-num-gpus` is ignored; see | ||
| [Training Backends](/user-guide/training-backend#3-choosing-the-gpu-layout). |
There was a problem hiding this comment.
🟡 This PR adds a new ## misc_args Argument Groups section (docs/user-guide/argument-groups.md:176-195, with anchor #misc-args), but doesn't update docs/user-guide/launch-script.md:145-148, which still says "Two blocks have no Argument Groups section: misc_args carries the cluster shape ... and the wandb flags..." — that claim is now false, since misc_args has its own section. Fix: update the sentence to name only the wandb block, and add a misc_args row to the preceding "Block -> Flags documented at" table (launch-script.md:136-143).
Extended reasoning...
docs/user-guide/launch-script.md (not touched by this PR) contains a "Block → Flags documented at" table (lines 136–143) followed by this sentence at lines 145–148:
Two blocks have no Argument Groups section:
misc_argscarries the cluster shape (--colocate,--actor-num-nodes,--actor-num-gpus-per-node), and the wandb flags come fromU.get_default_wandb_args, which returns them only whenWANDB_API_KEYis set...
This PR adds a brand-new ## misc_args - GPU layout and everything else section to docs/user-guide/argument-groups.md (diff lines 176–195), complete with an <a id="misc-args"></a> anchor and a table row documenting --actor-num-nodes, --actor-num-gpus-per-node, --colocate, and the other flags launch-script.md attributes to the "undocumented" misc_args block. The PR is thorough about propagating this new group elsewhere — concepts.md, developer/contributor-guide.md, and models/kimi/kimi-k2.5.md all gained a misc_args row or reference — but it missed updating launch-script.md's explicit negative claim, which it directly falsifies.
Why existing code/docs don't catch this: launch-script.md is a separate file with no automated cross-reference check tying its prose claims to the section list in argument-groups.md. The PR's own stated theme is fixing "commands that fail as written... symbols that no longer exist... values that disagree with the code" via a careful audit, and it explicitly reworked argument-groups.md's framing (bash arrays → Python launcher variables) while cross-updating every other page that names the argument groups — so this is a straightforward miss in an otherwise systematic pass, not a design decision.
Concrete proof, step by step:
- Before this PR:
argument-groups.mdhad 8 sections (MODEL_ARGSthroughSGLANG_ARGS), none namedmisc_args.launch-script.md's claim "two blocks have no Argument Groups section:misc_args... and the wandb flags" was accurate. - This PR adds a 9th section,
## misc_args - GPU layout and everything else, with anchor#misc-args, documenting exactly the flags (--colocate,--actor-num-nodes,--actor-num-gpus-per-node, etc.) thatlaunch-script.mdsays have no section. - Reading
launch-script.mdtoday (verified viased -n '130,150p'on the current checkout — the PR does not touch this file) still shows the same "Two blocks have no Argument Groups section:misc_args..." sentence, and the preceding table still lists only 7 blocks with nomisc_argsrow. - A reader who follows
launch-script.md's claim will incorrectly believemisc_argsflags are documented nowhere, whenargument-groups.md#misc-argsnow covers them — the opposite of what this PR intends by cross-linking every other page to the new section.
Fix: in docs/user-guide/launch-script.md, add a misc_args row to the "Block → Flags documented at" table (pointing at /user-guide/argument-groups#misc-args), and rewrite the sentence to say only the wandb block lacks a dedicated section (e.g. "One block has no Argument Groups section: the wandb flags come from U.get_default_wandb_args...").
This is docs-only with no runtime/behavioral impact, so it's a nit, not a blocking issue — but it directly contradicts a factual claim this same PR just made false, which is exactly the class of staleness the PR is otherwise trying to eliminate.
| ``` | ||
|
|
||
| The launcher sets the required env vars for you: `SGLANG_SKIP_CHECKPOINT_LOAD_CHECK=1`, `SGLANG_DSV4_FP4_EXPERTS=0`, `MILES_HACK_TRAIN_TORCH_DETERMINISTIC=1`, and `NCCL_ALGO=Ring`. | ||
| The launcher sets the required env vars for you: `SGLANG_SKIP_CHECKPOINT_LOAD_CHECK=1`, `SGLANG_DSV4_FP4_EXPERTS=0`, `SGLANG_HEALTH_CHECK_TIMEOUT=120`, `SGLANG_DG_CACHE_DIR_PER_PROCESS=1`, and `SGLANG_OPT_FP8_WO_A_GEMM=0`. Passing `--train-deterministic` adds `NCCL_ALGO=Ring`, `NVTE_ALLOW_NONDETERMINISTIC_ALGO=0`, and `CUBLAS_WORKSPACE_CONFIG=:4096:8` on top of those. |
There was a problem hiding this comment.
🟡 The doc says "Passing --train-deterministic adds NCCL_ALGO=Ring, NVTE_ALLOW_NONDETERMINISTIC_ALGO=0, and CUBLAS_WORKSPACE_CONFIG=:4096:8 on top of those" — but in scripts/run_deepseek_v4.py, train_deterministic defaults to True, so these three env vars (plus --deterministic-mode) are set in every stock run, not just when the flag is explicitly passed. A user would need --no-train-deterministic to remove them. The same misleading framing is repeated on the V4-Pro page.
Extended reasoning...
The bug: docs/models/deepseek/deepseek-v4-flash.md line 173 reads: "The launcher sets the required env vars for you: ... Passing --train-deterministic adds NCCL_ALGO=Ring, NVTE_ALLOW_NONDETERMINISTIC_ALGO=0, and CUBLAS_WORKSPACE_CONFIG=:4096:8 on top of those." The word "Passing" frames --train-deterministic as an opt-in flag that is off unless the user explicitly adds it to their command line.
Why that's backwards: In scripts/run_deepseek_v4.py, ScriptArgs declares train_deterministic: bool = True as the dataclass default (alongside the docstring-adjacent "precision configs" block). The launcher is a Typer app built from that dataclass, so a bool field defaulting to True becomes a --train-deterministic / --no-train-deterministic pair whose effective default is on. The three env vars, plus --deterministic-mode, are gated purely on if args.train_deterministic: — a condition that is true for every stock invocation of full-train or train unless the user actively passes --no-train-deterministic.
The code path that triggers it: Any reader who runs the one-line launch command shown earlier on the same page (python scripts/run_deepseek_v4.py full-train --model-name DeepSeek-V4-Flash-FP8 ...) gets all eight env vars — the five unconditional ones plus these three — without ever touching --train-deterministic. The doc's framing would lead that same reader to conclude, incorrectly, that only the first five are active in their run.
Why nothing else catches this: This is a pure prose/code disagreement — there's no test or lint that checks doc text against a launcher's dataclass defaults, so the mismatch survives silently. It was introduced in this very PR when the sentence was rewritten to add the three previously-undocumented vars (replacing a doc line that referenced a nonexistent MILES_HACK_TRAIN_TORCH_DETERMINISTIC var) but kept the old opt-in phrasing structure.
Step-by-step proof:
- Open
scripts/run_deepseek_v4.py; findtrain_deterministic: bool = TrueinScriptArgs. - Find the block
if args.train_deterministic: misc_args += "--deterministic-mode "; extra_env_vars |= {"NCCL_ALGO": "Ring", "NVTE_ALLOW_NONDETERMINISTIC_ALGO": "0", "CUBLAS_WORKSPACE_CONFIG": ":4096:8"}. - Since the dataclass default is
Trueand Typer exposes it as a flag pair defaulting to the dataclass value, runningpython scripts/run_deepseek_v4.py full-train --model-name DeepSeek-V4-Flash-FP8 --num-nodes 8 --num-gpus-per-node 8(the exact command shown earlier in this same doc page, §3.1) sets all three vars and--deterministic-mode, with zero mention of--train-deterministicon the command line. - A reader trusting the doc's "Passing
--train-deterministicadds ... on top of those" would instead expect these three vars to be absent in that exact command, which is the opposite of what actually happens.
The fix: Reword to something like: "The launcher sets the required env vars for you: SGLANG_SKIP_CHECKPOINT_LOAD_CHECK=1, ..., and (by default, via --train-deterministic) NCCL_ALGO=Ring, NVTE_ALLOW_NONDETERMINISTIC_ALGO=0, and CUBLAS_WORKSPACE_CONFIG=:4096:8; pass --no-train-deterministic to disable the latter three." The same fix applies to the parallel sentence on docs/models/deepseek/deepseek-v4-pro.md ("Required env vars ... and the Pro-only pair ..."), which inherits the same launcher default and is not itself opt-in-framed but should be checked for the same assumption if it implies these are conditional.
|
|
||
| These metrics track importance sampling weights and corrections. They are only computed when `--use-tis` is enabled. | ||
|
|
||
| When using `--custom-tis-function-path` pointing to MIS implementation (e.g., `mis.py`), additional fine-grained metrics become available: | ||
|
|
||
| | Metric Name | Description | Required Args | Optional Control Args | | ||
| |------------|-------------|---------------|----------------------| | ||
| | `ois` | On-policy importance sampling ratio: $\exp(\log \pi_{\text{train}} - \log \pi_{\text{old}})$ | `--use-tis` | Only for Algorithm 2 (Decoupled PPO) | | ||
| | `mis_mean_is_weight_before_clip` | Raw IS weights before any correction: $\exp(\text{log-ratio})$ | `--use-tis` | `--mis-level` (token/sequence/geometric) | | ||
| | `mis_ratio_mean_after_mis` | IS weights after correction (bounded or masked) | `--use-tis` | `--mis-mode`, bounds | | ||
| | `mis_truncate_fraction` | Fraction of weights truncated (mode-specific) | `--use-tis`, `--mis-mode=truncate` | `--mis-upper-bound` | | ||
| | `mis_clip_fraction_low` | Fraction of weights clipped below lower bound | `--use-tis`, `--mis-mode=clip` | `--mis-lower-bound`, `--mis-upper-bound` | | ||
| | `mis_clip_fraction_high` | Fraction of weights clipped above upper bound | `--use-tis`, `--mis-mode=clip` | `--mis-lower-bound`, `--mis-upper-bound` | | ||
| | `mis_mask_fraction_low` | Fraction of tokens rejected (below lower bound) | `--use-tis`, `--mis-mode=mask` | `--mis-lower-bound`, `--mis-upper-bound` | | ||
| | `mis_mask_fraction_high` | Fraction of tokens rejected (above upper bound) | `--use-tis`, `--mis-mode=mask` | `--mis-lower-bound`, `--mis-upper-bound` | | ||
| | `mis_catastrophic_token_fraction` | Fraction of catastrophic tokens (veto-specific) | `--use-tis`, `--mis-veto-threshold` set | Sequence-level rejection | | ||
| | `mis_catastrophic_seq_fraction` | Fraction of sequences with catastrophic tokens | `--use-tis`, `--mis-veto-threshold` set | Sequence-level rejection | | ||
| | `mis_batch_norm_factor` | Batch normalization factor applied to weights | `--use-tis`, `--mis-batch-normalize` | Normalizes mean to 1.0 | | ||
| When using `--custom-tis-function-path` pointing to MIS implementation (e.g., `mis.py`), additional fine-grained metrics become available. The `tis_` and `rs_` prefixes say which stage produced the number. | ||
|
|
||
| | Metric Name | Description | Emitted when | | ||
| |------------|-------------|--------------| | ||
| | `ois` | On-policy importance sampling ratio: $\exp(\log \pi_{\text{train}} - \log \pi_{\text{old}})$ | `--use-tis` (Algorithm 2 only) | | ||
| | `tis_weight_before_bound` | Raw IS weights before any bounding: $\exp(\text{log-ratio})$ | `use_tis` | | ||
| | `tis_weight_after_bound` | IS weights after the `tis_mode` bounding | `use_tis` | | ||
| | `tis_truncate_fraction` | Fraction of weights truncated | `tis_mode: truncate` | | ||
| | `tis_clip_fraction_low` / `tis_clip_fraction_high` | Fraction of weights clipped below / above the bound | `tis_mode: clip` | | ||
| | `tis_mask_fraction_low` / `tis_mask_fraction_high` | Fraction of tokens rejected below / above the bound | `tis_mode: mask` | | ||
| | `rs_mask_fraction_low` / `rs_mask_fraction_high` | Fraction of tokens rejected by rejection sampling | `use_rs` | | ||
| | `rs_catastrophic_token_fraction` | Fraction of catastrophic tokens (below the veto threshold) | `rs_veto_threshold` set | | ||
| | `rs_catastrophic_seq_fraction` | Fraction of sequences holding a catastrophic token | `rs_veto_threshold` set | | ||
| | `is_ratio_mean_after_tis_rs` | Mean IS weight after both TIS and RS | `use_tis` | | ||
| | `batch_norm_factor` | Batch normalization factor applied to weights (1.0 when off) | `use_tis` | | ||
| | `is_ratio_mean_final` / `is_ratio_min_final` / `is_ratio_max_final` | Final IS weight statistics actually multiplied into the loss | `use_tis` | | ||
|
|
||
| ## Reference | ||
|
|
There was a problem hiding this comment.
🟡 The rewritten Mismatch/IS-RS metric tables list bare names (training_log_ppl, kl, tis_weight_before_bound, is_ratio_mean_final, etc.), but the doc's own recommended wiring (--custom-tis-function-path ...mis.compute_mis_weights_with_cp) prefixes every one of these with mis_ before they reach wandb (mis.py: key_name = f"mis_{key}", unchanged by losses.py/log_utils.py), so the real names are train/mis_training_log_ppl, train/mis_kl, etc. The Mismatch Monitoring table's claim that those metrics are computed 'regardless of whether TIS/MIS correction is enabled' is also wrong for the built-in path: the default vanilla_tis_function only emits tis/tis_clipfrac/tis_abs. Mirrored in examples/infra_features/train_infer_mismatch_helper/README.md at the same tables.
Extended reasoning...
This PR's stated goal for this section was to fix wrong metric names ("The mis_* / mismatch_* metric names in the rollout-correction example match nothing in mis.py. Replaced with the names actually emitted..."), but the replacement still doesn't match what actually reaches wandb once you follow the doc's own wiring instructions.
The code path. The two tables (Mismatch Monitoring Metrics at ~L190-210 and IS/RS Correction Metrics at ~L212-233) list bare keys — training_log_ppl, rollout_log_ppl, kl, k3_kl, chi2_token, tis_weight_before_bound, tis_truncate_fraction, rs_catastrophic_token_fraction, is_ratio_mean_final, batch_norm_factor, etc. — as the literal wandb metric names. These names do originate as dict keys inside mis.py's compute_mis_weights (via add_ppl_metrics and the IS/RS bookkeeping). But the doc's own "Config keys" section, just above, tells the reader to wire the feature up with:
--custom-tis-function-path examples.infra_features.train_infer_mismatch_helper.mis.compute_mis_weights_with_cp
compute_mis_weights_with_cp (and the FSDP counterpart compute_mis_weights_fsdp) do not return the raw dict — they wrap every key before returning: key_name = f"mis_{key}". From there, miles/backends/training_utils/loss_hub/losses.py copies tis_metrics into reported_loss verbatim (key_name = f"{metric_key}", no stripping), and miles/backends/training_utils/log_utils.py logs them to wandb as train/{key} unchanged. So with the exact wiring the doc recommends, a reader who greps their wandb dashboard for train/training_log_ppl or train/tis_truncate_fraction finds nothing — the real names are train/mis_training_log_ppl, train/mis_kl, train/mis_tis_truncate_fraction, train/mis_is_ratio_mean_final, and so on for every row in both tables.
Step-by-step proof:
- User follows the doc's "Config keys" example and passes
--custom-tis-function-path ...mis.compute_mis_weights_with_cp --use-tis. - Inside
mis.py,compute_mis_weightsbuilds a metrics dict with keys liketraining_log_pplandtis_weight_before_bound(viaadd_ppl_metricsand the TIS/RS bookkeeping code). compute_mis_weights_with_cp(the function the doc told you to point at) wraps the return:{f"mis_{key}": v for key, v in metrics.items()}(mirrored incompute_mis_weights_fsdp).losses.pymerges this dict intoreported_losswith no key transformation.log_utils.pylogs each entry astrain/{key}.- Net result on wandb:
train/mis_training_log_ppl, nottrain/training_log_pplas the doc's table states.
Why existing review/tests don't catch it: this is a documentation-only PR (verified: mis.py, losses.py, log_utils.py are untouched by the diff), and there's no test that asserts docs match emitted metric names — the mismatch is purely between the table text and the code the doc itself points at.
Secondary claim: the Mismatch Monitoring table also states these metrics "are computed automatically when rollout_log_probs are provided, regardless of whether TIS/MIS correction is enabled." That's only true when a custom TIS function is wired in. The built-in vanilla_tis_function (miles/backends/training_utils/loss_hub/corrections.py) computes only tis, tis_clipfrac, tis_abs — it never computes training_log_ppl/kl/etc. So on the built-in path (no --custom-tis-function-path), none of the named metrics in that table appear at all, prefixed or not.
Suggested fix: either prefix every row in both tables with mis_ (to match compute_mis_weights_with_cp/compute_mis_weights_fsdp), or add a note that the bare names apply only to a custom function that returns them unprefixed, and that the shipped reference implementation (mis.py) always adds the mis_ prefix. Also soften "regardless of whether TIS/MIS correction is enabled" to clarify it's true only for rollout_log_probs-derived metrics computed by the loss hub itself, not for the mis.py-sourced ones which require both --use-tis and a custom TIS function path.
This is docs-only with no runtime impact, but it's precisely the class of error this PR set out to fix, so worth correcting in the same pass. Mirrored verbatim in examples/infra_features/train_infer_mismatch_helper/README.md, which should get the same fix.
| | `eval/skipped_busy` | At `--eval-max-in-flight` under `--eval-overflow-policy skip` | | ||
| | `eval/skipped_export_failed` | The snapshot export raised | | ||
| | `eval/skipped_ckpt_missing` | No `.complete` marker in the snapshot directory | | ||
| | `eval/skipped_unhealthy` | The fleet or its router was unreachable | | ||
| | `eval/skipped_pin_violation` | The engines did not all report the expected weight version | | ||
| | `eval/skipped_crashed` | Anything else the eval raised | | ||
| | `eval/skipped_<reason>` | A `CheckpointEvalFn` raised `EvalSkip(reason)` | | ||
|
|
||
| For a point that did run, `eval/{dataset}/weight_version/mean == eval/step` and | ||
| `eval/{dataset}/weight_version/mixed_version_ratio == 0` together confirm it measured |
There was a problem hiding this comment.
🟡 The removed eval/skipped_unhealthy and eval/skipped_pin_violation rows are still accurate — EvalFleet.pin() in miles/ray/rollout/eval_fleet.py still raises EvalSkip("unhealthy") and EvalSkip("pin_violation") whenever --eval-num-gpus > 0 (the Dedicated fleet backend documented earlier in this same file), and rollout_manager.py's _eval_checkpoint still logs them under exactly those metric names. The new catch-all row also wrongly attributes them to a user-supplied CheckpointEvalFn when they actually come from the built-in EvalFleet.
Extended reasoning...
The PR's stated premise for deleting these two rows is that eval/skipped_unhealthy and eval/skipped_pin_violation have no emitter. That premise is false for the Dedicated-fleet eval backend, which this very doc documents two sections below ("Mode 2: Dedicated fleet", enabled via --eval-num-gpus N).
The code path
miles/ray/rollout/eval_fleet.py's EvalFleet.pin() raises EvalSkip("unhealthy") in two places (when the router or engines fail a readiness probe) and EvalSkip("pin_violation") when _pin_fleet reports the engines did not all converge on the expected weight version. rollout_manager.py's _eval_checkpoint calls self._eval_fleet.pin(...) and catches EvalSkip generically, forwarding e.reason to report_eval_skip → log_eval_skip, which logs f"eval/skipped_{reason}". Reason strings "unhealthy" and "pin_violation" therefore surface as eval/skipped_unhealthy and eval/skipped_pin_violation exactly as the deleted rows described — and the deleted rows' descriptions ("the fleet or its router was unreachable" / "the engines did not all report the expected weight version") match pin()'s behavior precisely.
Why nothing else in this PR catches it
The PR's own audit methodology (grep for the metric string, confirm no emitter) apparently missed eval_fleet.py because the emission is indirect: the string is built dynamically as f"eval/skipped_{reason}" in a shared helper, not written literally as eval/skipped_unhealthy anywhere. A textual audit of the doc's exact metric names against the codebase would not surface the fact that "unhealthy" and "pin_violation" are two of the possible values substituted into that f-string.
Impact
This is a real behavior/documentation mismatch: an operator running the Dedicated-fleet eval backend (--eval-num-gpus > 0) who sees eval/skipped_unhealthy or eval/skipped_pin_violation in their metrics will find no explanation for it in this table, and may instead misread the new catch-all row (eval/skipped_<reason> | A CheckpointEvalFn raised EvalSkip(reason)) as meaning they have a custom CheckpointEvalFn configured, which they may not.
Step-by-step proof
- Run with
--eval-num-gpus 1 --eval-interval K(Dedicated fleet mode, per this doc's "Mode 2" section). - At an eval-due step,
_eval_checkpointcallsawait self._eval_fleet.pin(hf_dir, version). - Inside
pin(), if the fleet's router does not report ready in time, it raisesEvalSkip("unhealthy"); if the engines pin to the wrong weight version, it raisesEvalSkip("pin_violation"). _eval_checkpoint'sexcept EvalSkip as e:catches it and callsreport_eval_skip(rollout_id, e.reason).log_eval_skiplogs{f"eval/skipped_{e.reason}": 1}, i.e.{"eval/skipped_unhealthy": 1}or{"eval/skipped_pin_violation": 1}.- These exact metric names appear in wandb/logs, but per this PR's edit, are undocumented and their only textual match in the table is the generic
CheckpointEvalFnrow, which misattributes the source.
Fix
Restore the two specific rows (or fold them into a fleet-specific note) rather than deleting them, and keep the generic CheckpointEvalFn row separate since it describes a different code path (a user's custom eval function, not the built-in EvalFleet).
On the refutations
All three verifiers independently confirmed this via direct source inspection, and none refuted it — this is a straightforward, uncontested finding.
nblintao
left a comment
nblintao
left a comment
There was a problem hiding this comment.
My agent found 2 mistakes that seems reasonable to me. But I don't have full context. Could you check if it makes sense?
| | `eval/skipped_unhealthy` | The fleet or its router was unreachable | | ||
| | `eval/skipped_pin_violation` | The engines did not all report the expected weight version | |
There was a problem hiding this comment.
These two do have emitters — eval_fleet.py raises EvalSkip("unhealthy") / EvalSkip("pin_violation") and metrics.py logs eval/skipped_{reason}, so the literal names never show up in a grep. I'd keep both rows and add the generic one on top.
| | `tis_weight_before_bound` | Raw IS weights before any bounding: $\exp(\text{log-ratio})$ | `use_tis` | | ||
| | `tis_weight_after_bound` | IS weights after the `tis_mode` bounding | `use_tis` | | ||
| | `tis_truncate_fraction` | Fraction of weights truncated | `tis_mode: truncate` | | ||
| | `tis_clip_fraction_low` / `tis_clip_fraction_high` | Fraction of weights clipped below / above the bound | `tis_mode: clip` | | ||
| | `tis_mask_fraction_low` / `tis_mask_fraction_high` | Fraction of tokens rejected below / above the bound | `tis_mode: mask` | | ||
| | `rs_mask_fraction_low` / `rs_mask_fraction_high` | Fraction of tokens rejected by rejection sampling | `use_rs` | | ||
| | `rs_catastrophic_token_fraction` | Fraction of catastrophic tokens (below the veto threshold) | `rs_veto_threshold` set | | ||
| | `rs_catastrophic_seq_fraction` | Fraction of sequences holding a catastrophic token | `rs_veto_threshold` set | | ||
| | `is_ratio_mean_after_tis_rs` | Mean IS weight after both TIS and RS | `use_tis` | | ||
| | `batch_norm_factor` | Batch normalization factor applied to weights (1.0 when off) | `use_tis` | | ||
| | `is_ratio_mean_final` / `is_ratio_min_final` / `is_ratio_max_final` | Final IS weight statistics actually multiplied into the loss | `use_tis` | |
There was a problem hiding this comment.
These still need the mis_ prefix: both compute_mis_weights_with_cp and compute_mis_weights_fsdp end with result_metrics[f"mis_{key}"], so wandb shows train/mis_tis_truncate_fraction, train/mis_is_ratio_mean_final, etc. Same for the PPL table above. Note batch_norm_factor was already correct as mis_batch_norm_factor before this PR.
…s_ prefix - launch-script said two blocks have no Argument Groups section; misc_args now has one, so add its row to the block table and leave only wandb - deepseek-v4 framed --train-deterministic as opt-in, but ScriptArgs defaults it to True, so a stock run already gets --deterministic-mode and the three env vars; say so and name --no-train-deterministic. Note the same default on the Pro page - the mis.py metric names needed the mis_ prefix its wrapper adds (mis.py:391 / :506), so the wandb keys are train/mis_training_log_ppl, train/mis_tis_truncate_fraction, ... The ois and train_rollout_logprob_abs_diff pair come from miles and stay unprefixed - the ppl / kl / chi2 metrics are not emitted "regardless of whether TIS/MIS correction is enabled": the built-in vanilla_tis_function reports only tis, tis_clipfrac and tis_abs, so they need the --custom-tis-function-path wiring
2 participants
Audit of
docs/againstmain. This PR only fixes claims that are wrong — commands that fail as written, symbols that no longer exist, and values that disagree with the code. Coverage gaps (flags that exist but are undocumented) are deliberately left out; see the follow-ups at the bottom.Commands that fail as written
models/glm/glm4-5,models/glm/glm4-7-flashpython3 scripts/model_args.py <type>python3 miles/utils/external_utils/model_args_utils.py <type>—scripts/model_args.pydoes not exist; the other 17 conversion snippets already use the right pathuser-guide/cli-referenceon_policy_distillationlisted as an--advantage-estimatorvaluearguments.py:1422has no such choice, and its own help says OPD is enabled on top of any estimator via--use-opdci/02-docker-build,developer/versions--variant rocm-mi300/rocm-mi350rocm700-mi30x/rocm700-mi35x/rocm720-mi35x— the real keys indocker/build.pyand thevariantchoice list indocker-build.yml.rocm700-mi35xwas missing from both tablesmodels/thinkingmachines/inkling§5.5--optimizer-state-nvme-dir,--optimizer-state-nvme-chunk-mbscripts/run_inkling.pypasses--offload-train-target disk --offload-train-disk-dirwhen colocated and--stream-optimizer-state-to-disk --offload-train-disk-dir --offload-train-disk-chunk-mb 256under--fully-async. Also split the grad-reduce line, which is--grad-reduce-in-bf16vs--accumulate-allreduce-grads-in-fp32on the same conditioncli-reference,training-backend,argument-groups,models/kimi/kimi-k2--sglang-enable-ep-moe,--sglang-enable-deepep,--sglang-enable-deepep-moe,--sglang-enable-overlap-scheduleServerArgs. Replaced with what the recipes actually pass:--sglang-ep-size(75 uses),--sglang-moe-a2a-backend(15),--sglang-moe-runner-backend(40),--sglang-deepep-mode(13)examples/infra_features/train_infer_mismatch_helper/README.md--tis-mode/--tis-level/--tis-*-bound/--tis-batch-normalize/--rs-*documented as CLI flags, plus a--use-rollout-correctionflagmis.yaml, reached through--custom-config-path+--custom-tis-function-path; the only CLI toggles are--use-tisand--use-rollout-logprobs.--use-rollout-correctiondoes not exist anywhere in the repo and was the header of an algorithm tableSymbols and paths that do not exist
miles/ray/rollout.pyis a package now.advanced/fault-tolerance(4×),advanced/pd-disaggregationandexamples/geo3k_vlm/README.mdnow point atrollout_manager.py,rollout_server.py,server_group.pyandtrain_data_conversion.pyrespectively. While there,fault-tolerancenow also mentions that the rollout paths needrolloutin--ft-components, not just--use-fault-tolerance.MILES_HACK_TRAIN_TORCH_DETERMINISTIC(models/deepseek/deepseek-v4-flash) does not exist.scripts/run_deepseek_v4.pysets five env vars unconditionally (SGLANG_SKIP_CHECKPOINT_LOAD_CHECK,SGLANG_DSV4_FP4_EXPERTS,SGLANG_HEALTH_CHECK_TIMEOUT,SGLANG_DG_CACHE_DIR_PER_PROCESS,SGLANG_OPT_FP8_WO_A_GEMM) and three more only under--train-deterministic(NCCL_ALGO,NVTE_ALLOW_NONDETERMINISTIC_ALGO,CUBLAS_WORKSPACE_CONFIG). The V4-Pro page got the same five.with_transformers_patch()/miles/utils/transformers_patch.pyare both gone; thedeepseek_ref→deepseek_v4handling is an in-placeconfig.jsonrewrite in the launcher.eval/skipped_unhealthy,eval/skipped_pin_violationhave no emitter. The built-in reasons arebusy,export_failed,ckpt_missing,crashed, plus whatever aCheckpointEvalFnpasses toEvalSkip(reason).mis_*/mismatch_*metric names in the rollout-correction example match nothing inmis.py. Replaced with the names actually emitted:tis_weight_before_bound,tis_truncate_fraction,tis_clip_fraction_low|high,tis_mask_fraction_low|high,rs_mask_fraction_low|high,rs_catastrophic_token_fraction,rs_catastrophic_seq_fraction,is_ratio_mean_after_tis_rs,batch_norm_factor,is_ratio_{mean,min,max}_final, and the unprefixedtraining_log_ppl/rollout_log_ppl/kl/k3_kl/log_ppl_diff/ppl_ratio/chi2_token/chi2_seq.miles/utils/wandb_utils.py→miles/utils/tracking_utils/wandb_utils.py.Wrong values and premises
--update-weight-buffer-sizedefaults to 512 MB, not 1 GB (arguments.py:773is512 * 1024**2).user-guide/argument-groupsopened with "Miles launch scripts are bash arrays."scripts/holds 34 Python launchers and zero.shfiles, and the page contradicteduser-guide/launch-script, which already describes the Python layout correctly. Rewritten around the<group>_argslocals the launchers actually build, with the previously-missingmisc_argsgroup (35 launchers use it) documented and the architecture flags explained as coming fromscripts/models/<megatron_model_type>.pyviaexecute_train. The<a id>anchors are unchanged, so incoming links still resolve;concepts,training-backend,user-guide/index,developer/contributor-guideandmodels/kimi/kimi-k2.5were updated to the same names.developer/architecturesent new reward types tomiles/rollout/sglang_rollout.py; therm_typedispatch lives inmiles/rollout/rm_hub/__init__.py, andrm_hub/was missing from the package tree.--rm-typeenumerations (cli-reference,customization) omittedgemma_math— whichscripts/run_gemma_4_*.pyandmodels/gemma/gemma-4both use, so readers would take it for an invalid value — plusdeterministic_randomand theboxed_prefix modifier.--rm-typeis also a freestr, not an enum.ci/01-label) listed 15 of the 19 entries inKNOWN_LABELS, omittingeval,fully-async,miles-pluginandamd.ci/00-stagealready tells readers howrun-ci-amdbehaves.Verification
pre-commit run --all-filesclean (includes the examples→docs mirror check).python scripts/tools/sync_example_docs.py --checkclean; the two generated pages were regenerated from their READMEs rather than edited directly.kimi-k3→ Kimi-K3 lora RL day-0 support #1825,qwen3-8→ Qwen 3.8 day-0 lora RL support #2488), 0--sglang-*flags absent from the pinnedServerArgs.Deliberately not in this PR
Coverage and consistency work, worth separate PRs:
cli-referencesays it lists every Miles flag; it covers 91 of 344, and 133 registered flags appear nowhere in the docs — including all of MLflow (--use-mlflow+ 4), TensorBoard (--use-tensorboard+ 2) and Prometheus (--prometheus-port+ 1).user-guide/monitoringdocuments only wandb as a result.customizationsays it lists every hook; sixload_functionhooks are missing (--custom-async-data-buffer-path,--custom-update-weight-post-write-path,--generate-{execute-tool-function,tool-specs}-path,--session-sample-{picker,postprocessor}-path).docker/npu_patch/,scripts/run_qwen3_4b_npu.pyandexecute_train_npu(), but no page mentions it;indexandinstallationlist only NVIDIA and AMD, and disagree onMI350vsMI350X./advanced/architecture-supportand survive only on thedocs.jsonredirect, with link text that no longer names the destination.SGLANG_ARGS=(...)shell blocks, a leftover of the same bash-array framing this PR removed fromargument-groups.low-precisionwritesGroupLinear/GroupGEMMwhereint4-qatwrites TEGroupedLinear; a stray empty## TODOsection inexamples/infra_features/low_precision/README.md; an unrendered{/* FIGURE PLACEHOLDER */}inlaunch-script; and five British spellings.