Warn when a string model loads in float32 under mixed-precision training - #6005
Warn when a string model loads in float32 under mixed-precision training#6005behroozazarkhalili wants to merge 22 commits into
Conversation
|
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. |
31198b0 to
05805f2
Compare
|
Thanks, but the thing is that is doesn't explain why mixed precision training gives worse results, and whether it's a results that can be generalized |
|
I think we need a deep investigation on why, because it shouldn't be the case |
|
I spent some time on the "why, and does it generalize" question. Summary up front: the gap is real and it is a training-time numerics effect (not a data, masking, or weight-load bug), but it does not reproduce at small scale. Across three small models, three seeds, and two architectures, loading in fp32 is consistently a touch better than bf16, which is the opposite of the +8.9 you measured at 3B. So the 3B effect looks scale-specific rather than a universal fp32-vs-bf16 rule. Details below. 1. What is and isn't happening (layer-by-layer trace, Llama-3.2-3B-Instruct)I traced both load paths step by step on a fixed batch, identical seed and data order, the only variable being the load dtype:
So at the per-step level the conventional expectation holds: fp32 is at least as accurate as bf16, and "it shouldn't be worse" is correct. The surprising part is purely the downstream training outcome at 3B. 2. Reproduction at small scale (full fine-tune, full GSM8k)A 3B full fine-tune fp32 arm did not fit the GPU memory available to me (full-parameter fp32 AdamW state exceeds my largest slice; see the limitation note), so I reproduced the A/B at <=1.2B where it fits, and added seeds and architectures to separate "noise" from "model-specific". Setup (identical between the two arms; only the load dtype changes):
Results (delta = bf16 - fp32, GSM8k test accuracy):
Every run is negative: fp32-default-load is consistently >= bf16-load, by roughly 1-3 points. The Llama-1B multi-seed mean is -1.97, so the small fp32 advantage is reproducible and not seed luck. It also holds on a different architecture (Qwen2.5-0.5B) and an independently trained model (SmolLM2-360M). 3. ReadingAt <=1.2B the effect is small and points the other way from the 3B result, and it lines up with the trace: fp32 is the more precise path, so at small scale it trains a touch better. The +8.9 bf16 advantage at 3B therefore does not generalize down to small scale; it inverts. That is consistent with this being a model-scale (and possibly architecture) specific mixed-precision sensitivity rather than a universal "bf16 trains better" rule. For TRL this argues for the conservative direction in PR #6005: emit a warning when a string model loads in fp32 under bf16/fp16 autocast (so the silent state is visible and users can opt into pure bf16 with Limitations (stated plainly)
Happy to run more seeds or models in this <=1.2B range, or to retry the 3B arm if there is a memory-efficient optimizer configuration considered valid for the comparison. |
No, see #5138 (comment)
So, we have a 9 pts difference when we only change the precision: #5138 (comment) |
|
You're right on both, thanks for the correction. I retract the DeepSpeed framing. Your 63.76 vs 72.63 is a clean non-DeepSpeed, precision-only ablation, and the result you linked is non-DeepSpeed too. The 9 points at 3B come from the precision change alone, no ZeRO-3 involved. I am not disputing that. The +9 for bf16 at 3B, precision-only, is real. My small-scale runs were meant to answer the "does it generalize" part, not to argue against your 3B number, and the honest result is that it does not generalize downward: at <=1.2B the same precision-only knob goes slightly the other way (fp32 ahead by ~1-2 points across Llama-3.2-1B three seeds, Qwen2.5-0.5B, SmolLM2-360M). So the effect is real and large at 3B and small-and-reversed at <=1.2B, which reads as scale-dependent rather than universal. That does not weaken the case for the warning in this PR, it sharpens it: at 3B a user clearly benefits from bf16, and the silent fp32 load is actively costing them 9 points, so surfacing it is worth doing. The reason I would still stop short of flipping the default is only the small-scale direction, where the default flip would cost a point or two. But if you weight 3B-and-up as the case that matters (reasonable, since that is where real training happens), defaulting the string-model load to bf16 when One thing I could not do is reproduce your 3B number directly: a 3B full fine-tune fp32 arm does not fit the GPU memory I have. If it is useful I can run the 3B A/B with a memory-efficient optimizer (8-bit AdamW, applied identically to both arms) to check whether the +9 survives on hardware that fits, with the caveat that it is not vanilla AdamW so the absolute numbers may shift. Let me know if that is worth running, or if you would prefer I just switch the PR from a warning to defaulting the load to bf16. |
|
This discussion sounds a lot like llm sycophancy... |
|
@qgallouedec Experimental setupModel / data
Training (identical across every cell, single variable per cell)
Evaluation (your script, unchanged)
Replication scope: the full 3×2×3 grid ( Base model, through this exact pipeline
Your exact config (
|
| base | fixed fp32 SFT | |
|---|---|---|
| you reported | 66.26 | 63.76 (−2.5, below base) |
| I measure | 68.01 | 965/1319 = 73.16 (+5.2, above base) |
I do not reproduce a below-base result at your config. To rule out "lr=5e-6 is simply too high at 3B," I swept LR (correct, fp32, seed 42):
| LR | GSM8k | vs base |
|---|---|---|
| 1e-6 | 942/1319 = 71.42 | +3.41 |
| 5e-6 | 953/1319 = 72.25 | +4.24 |
| 1e-5 | 930/1319 = 70.51 | +2.50 |
All above base; none below.
Full grid — every cell, every seed (correct/1319)
| cell | seed 42 | seed 0 | seed 123 | mean | std | vs base |
|---|---|---|---|---|---|---|
| correct, fp32 | 948 = 71.87 | 924 = 70.05 | 960 = 72.78 | 71.57 | 1.13 | +3.56 |
| +double-BOS, fp32 | 991 = 75.13 | 984 = 74.60 | 967 = 73.31 | 74.35 | 0.76 | +6.34 |
| +ignore-EOS, fp32 | 946 = 71.72 | 923 = 69.98 | 954 = 72.33 | 71.34 | 1.00 | +3.33 |
| correct, bf16 | 918 = 69.60 | 915 = 69.37 | 927 = 70.28 | 69.75 | 0.39 | +1.74 |
| +double-BOS, bf16 | 958 = 72.63 | 965 = 73.16 | 953 = 72.25 | 72.68 | 0.37 | +4.67 |
| +ignore-EOS, bf16 | 918 = 69.60 | 929 = 70.43 | 924 = 70.05 | 70.03 | 0.34 | +2.02 |
Every cell, including both correct cells, is above base (68.01).
Effect of each toggle (vs the matching correct cell)
- double-BOS: fp32 lifts +3.26 / +4.55 / +0.53 (seeds 42/0/123); bf16 +3.03 / +3.79 / +1.97. Pooled: mean +2.85, std 1.30, range [+0.53, +4.55] — positive in all 6 conditions, but high seed variance (one fp32 seed is only +0.53, within noise).
- ignore-EOS: fp32 −0.15 / −0.07 / −0.45; bf16 0.00 / +1.06 / −0.23. Pooled: mean +0.03, std 0.48 — inert.
- precision (correct fp32 − correct bf16): +2.27 / +0.68 / +2.50 by seed, mean +1.82 (fp32 marginally above bf16; same direction at 1B, +1.36).
Your four claims, tested
| claim (from your tables) | result |
|---|---|
| fixed v0.29 fp32 SFT lands below base | does not reproduce — correct is +3.56 (fp32) / +1.74 (bf16) above base, at every seed |
| bf16 ≫ fp32 (you: +8.87) | does not reproduce — fp32 is marginally above bf16 (mean +1.82), opposite sign |
| ignore-EOS helps (you: +3.5 / +5.2) | does not reproduce — inert (+0.03 ± 0.48) |
| double-BOS helps (you: +1.67) | reproduces — +2.85 ± 1.30 over correct, positive in all 6 conditions, but seed-variable |
Only double-BOS slightly helping replicates. And even there, fixing it moves ~74 → ~72: a real drop, but above-base → above-base, not into harm.
Direct answer to "why fixing the bug gives worse results"
Across 3 seeds and both precisions I find no configuration where the fixed model lands below base — correct ranges 69.37–72.78, always above 68.01. There is therefore no "fix → worse than the base model" effect here to attribute a mechanism to. The single below-base number (your 63.76) does not reappear in any of my fp32 configs (3 LRs + your exact eff_batch=8), which span 70.51–73.16.
What I could not match (so I am not claiming your number is wrong)
I could not control: your exact trl / transformers / accelerate versions; whether the 63.76 run used DataCollatorForCompletionOnlyLM (the v0.13 path) or the v0.29 prompt–completion path; your eval checkpoint (checkpoint-901 vs my final save); and run-to-run variance — each of your table rows is a single run, no seeds, and at 3B I see ~1 point of seed noise.
Could you share the exact versions and the eval checkpoint used for the 63.76 row (and confirm which collator it used)? That is most likely where the difference lives.
Note on #6005: that PR only adds a warning when a string model loads in float32 under bf16/fp16 autocast — it surfaces a silent precision condition. It stands on its own regardless of the GSM8k numbers above; it does not claim the precision changes the score.
…training (#5138) When a model is passed to a trainer as a string, it is loaded via create_model_from_path, which defaults dtype to "float32". With bf16/fp16 autocast enabled (the default), this silently yields float32 master weights under autocast — measurably different from pure low-precision training and the root cause of the GSM8k regression reported in #5138. Add a shared helper warn_if_fp32_with_mixed_precision that emits a one-line hint pointing users to model_init_kwargs={"dtype": ...} when mixed precision is on and no explicit dtype was set. The gap is shared by every trainer that loads a string model through create_model_from_path, so the helper is called at the policy-model load site in SFT, DPO, GRPO, RLOO, Reward, and experimental KTO. It is intentionally not applied to frozen reference models.
4dffddd to
193e1e6
Compare
The fp32-under-mixed-precision warning named the active precision correctly (fp16/bf16) but always suggested `dtype: "bfloat16"`, steering fp16 users toward a mismatched dtype. Derive the suggested dtype from the same flag so fp16 suggests "float16" and bf16 suggests "bfloat16".
Resolves the conflicts introduced while this branch was behind main:
- The five trainers that carry the fp32 warning (SFT, DPO, GRPO, RLOO, Reward) all conflicted
on the same line: main replaced `args.model_init_kwargs or {}` with a defensive copy plus
`quantization_config` handling. Both changes are wanted, so the warning now runs after that
block and inspects the final `model_init_kwargs`.
- `trl/experimental/kto/kto_trainer.py` was deleted in main, but by promotion rather than
removal (#6175 moved it to `trl/trainer/kto_trainer.py`). Accepting the deletion alone would
have silently dropped KTO's fp32 warning while its five sibling trainers kept theirs, so the
warning is re-applied at the promoted location.
- `tests/test_utils.py` conflicted only on neighboring imports; both are kept.
The warning block is byte-identical across all six trainers.
…n support Both tests hardcoded `bf16=True` behind `require_torch_accelerator`, so on a pre-Ampere accelerator `SFTConfig` raises "Your setup doesn't support bf16/gpu" before the warning can be asserted. `warn_if_fp32_with_mixed_precision` fires on bf16 or fp16, so select whichever the device supports rather than dropping to no mixed precision at all, which would leave both assertions holding vacuously.
DistillationTrainer moved from trl/experimental/distillation/ to trl/trainer/ when it was promoted to the stable API, which makes it the seventh stable trainer loading a policy model through create_model_from_path. It was the only one not covered by the warning. Add the call at the student load, which is the trainable model, matching the placement and comment used by the other six trainers. The teacher load is left alone: it is frozen and never optimized, so a float32 teacher raises no mixed-precision training concern, as the helper's docstring already states. Verified: all seven stable trainers that call create_model_from_path now warn exactly once, and in distillation the warning precedes the student load while the teacher load stays unwarned.
Every call site injects `quantization_config` into `model_init_kwargs`
two lines before calling the helper, but the helper only checked whether
`dtype` was set. A QLoRA-style load therefore got told the model "will be
loaded in float32" and was pointed at `model_init_kwargs={"dtype": ...}`.
Both halves of that are wrong under quantization. The weights are stored
in the quantized format (nf4 for a 4-bit load) rather than in `dtype`, and
the compute precision comes from the quantization config itself via
`bnb_4bit_compute_dtype`, so `dtype` is not the lever the message
recommends.
Skip the warning when a `quantization_config` is present, which matches
how conservative the helper already is about not second-guessing an
explicit user choice.
Verified: before the change the warning fired identically for a 4-bit and
an 8-bit config; after it, both are silent while every non-quantized case
is unchanged. Covered by a new parametrized case.
|
@qgallouedec picking this back up, and keeping it short this time. Your "why" question is unresolved and I am not going to relitigate it here. The only thing I still need from you for it is the exact Separately, and this is the actual ask: this PR does not depend on that question. It adds a warning when a string model would load in float32 under bf16/fp16 autocast. It makes no claim about which precision trains better. Whatever the 3B result turns out to be, the load is currently silent, and What changed since you last looked:
So it is 10 files, +148/-1, of which the warning itself is one helper plus a three-line call in each trainer, and the rest is tests. Happy to drop it if you would rather not carry the surface. But if the silent fp32 load is worth surfacing at all, this is decidable on its own without waiting on the GSM8k thread. |
…6-warning # Conflicts: # trl/trainer/distillation_trainer.py
The branch was 48 commits behind and could not merge. The only conflict was in tests/test_utils.py, where both sides appended at the end of the file: main added a test_config_without_head_dim method to TestAdjustedMfu, and this branch added the TestWarnIfFp32WithMixedPrecision class. Both are kept, with main's method inside the class it belongs to and the new class after it. tests/test_utils.py -k "WarnIfFp32 or AdjustedMfu or ComputeFlopsPerToken or ComputeMfu" reports 19 passed on the merged tree, and ruff check and ruff format are clean at the pinned 0.13.3.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ac36b16. Configure here.
warn_if_fp32_with_mixed_precision keyed on the dtype key being absent from model_init_kwargs. Every TRL script forwards ModelConfig.dtype, whose default is "float32", so on any command-line run the key is present and the guard was always false. The warning never fired on the float32-plus-autocast configuration it exists to catch; it reached only library callers who omit the key entirely. The check now keys on the dtype the model will load in. "float32" and torch.float32 warn, "auto" and the low-precision dtypes stay silent, and a missing key still warns because create_model_from_path also defaults to float32. The message drops "because no dtype was specified", which is no longer what the guard tests. Two parametrized cases cover the explicit spellings. Both fail against the old guard and pass against the new one.
…experimental trainers
The warning also fired for fp16 and suggested loading the model in
float16. fp16 mixed precision runs a GradScaler, which refuses to
unscale float16 parameters ("Attempting to unscale FP16 gradients"),
so float32 master weights are the only working setup there and the
suggestion would have broken full fine-tuning. The warning is now bf16
only and suggests bfloat16 only.
A `quantization_config` key set to `None` counted as a quantized load
and silenced the warning; only a non-`None` value does now. `None` for
`dtype` is documented as deferring to the checkpoint, like `"auto"`,
and the one limitation that remains, a checkpoint quantized through its
own `config.json`, is stated in the docstring rather than guessed at.
IW-OPD, SDFT, SDPO, SSD and TPO load a string model through the same
branch as the seven core trainers and now warn the same way. AsyncGRPO
is left out: its config exposes `dtype` directly and writes it into
`model_init_kwargs` itself. The helper's docstring no longer lists
config names that go stale; it takes any `TrainingArguments`.
Tests follow: the fp16 case expects silence, `quantization_config=None`
expects the warning, a real `BitsAndBytesConfig` replaces the `object()`
placeholder the loader would have rejected, and the two SFT tests run
under bf16 only, skipping where the accelerator cannot do bf16.
The quantized row of the parametrization constructed `BitsAndBytesConfig(load_in_4bit=True)` at
collection time. On transformers 4.56.2, TRL's floor, that constructor needs bitsandbytes installed,
so a checkout without it could not collect `tests/test_utils.py` at all. The row now carries a marker
and the test builds the config itself after `pytest.importorskip("bitsandbytes")`, so the one row
skips where bitsandbytes is missing and the rest of the file still runs.

What
Adds a warning when a trainer loads a string model in float32 while bf16/fp16 mixed precision is enabled — the silent precision mismatch behind the SFT quality regression in #5138.
Why
When a model is passed as a path/identifier, it is loaded via
create_model_from_path, which defaultsdtypeto"float32". Withargs.bf16(orfp16) enabled — the default — a string model loads as float32 weights under bf16 autocast, which differs from pure low-precision training. In #5138 this was reproduced as a GSM8k regression that recovers when the model is loaded in bf16:The load path emits no warning, so the precision loss is silent and easy to miss.
What changed
warn_if_fp32_with_mixed_precision(args, model_init_kwargs)intrl/trainer/utils.py. It fires whenbf16is on and the model will load in float32: a missingdtype(whatcreate_model_from_pathdefaults to) or an explicit"float32", which is what every TRL script forwards fromModelConfig.dtype."auto",None, the low-precision dtypes and aquantization_configinmodel_init_kwargsstay silent, and so does fp16, whoseGradScalerrefuses float16 parameters. One known false positive: a checkpoint quantized through its ownconfig.jsonis not visible in the kwargs and still warns; the docstring says so.create_model_from_pathwith its float32 default. Left out on purpose: AsyncGRPO and AsyncDistillation defaultdtypeto"float32"deliberately and document why in their configs, so the warning would fire on every default run; A2PO loads throughfrom_pretraineddirectly, whose default dtype is transformers' own ("auto"on 5.x), so the helper's missing-key rule would misreport it.tests/test_utils.py) plus GPU-gated integration tests onSFTTrainer(tests/test_sft_trainer.py) — the warning fires on the default path and stays silent whendtypeis explicit.This addresses the "Problem A" (silent fp32 + autocast) half of #5138. The DeepSpeed ZeRO-3 collapse discussed later in the thread ("Problem B") is separate and not addressed here.
Resolves #5138
Note
Low Risk
Behavior change is log-only at trainer construction; no training or loading logic is altered beyond emitting a targeted warning.
Overview
Adds
warn_if_fp32_with_mixed_precisionintrl/trainer/utils.pyto surface a silent precision mismatch tied to #5138: withbf16=Trueand a string model path,create_model_from_patheffectively loads float32 weights under autocast unlessmodel_init_kwargsuses a non-fp32 dtype. The helper warns when the resolved load dtype is fp32 (including explicit"float32"fromModelConfig), suggestsmodel_init_kwargs={"dtype": "bfloat16"}, and skips fp16 training, quantized loads, and dtypes like"auto"/bfloat16.The call is wired right before string-model loading in SFT, DPO, GRPO, RLOO, KTO, Reward, Distillation, and several experimental trainers (IW-OPD, SDFT, SDPO, SSD, TPO)—not on frozen reference/teacher loads.
Tests: parametrized unit coverage in
tests/test_utils.pyand bf16-gatedSFTTrainerintegration tests that assert the warning appears on the default path and stays off when dtype is explicit.Reviewed by Cursor Bugbot for commit 2a3a16b. Bugbot is set up for automated code reviews on this repo. Configure here.