Repository navigation
feat(accel): opt-in cut cross-entropy (CCE) LM-head path (#588) - #657
DoubleCore wants to merge 2 commits into
Conversation
Signed-off-by: DoubleCore <doublecore@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaaae93ea1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if cut_cross_entropy: | ||
| return _cut_cross_entropy_logprobs(flat_hidden, positions, labels, lm_head, logit_softcap) |
There was a problem hiding this comment.
Cast deferred activations before the CCE matmul
When CCE is enabled for an untied Bailing V3 model at tp_size=1, the trunk returns BF16 activations while lm_head.weight is FP32 (areno/models/bailing_v3/model.py:1810-1814,1858-1864). This early return skips the existing chunk_hidden.to(lm_head.weight.dtype) conversion, and neither _prepare nor _chunk_row_stats casts the activations, so the first training step reaches a mixed-dtype matmul and raises. Normalize the hidden-state dtype before invoking CCE to preserve the previously supported FP32-head path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79b64b7eea57570821f1440b254d3170967e0a7b. The CCE handoff now casts selected hidden rows to the effective projection-weight dtype, so BF16 activations with an FP32 head no longer reach a mixed-dtype matmul.
Added test_packed_head_cut_cross_entropy_casts_hidden_to_fp32_weight, which compares selected scores and hidden/weight gradients with the materialized scorer. It raised the reported dtype error before this fix and now passes with torch 2.7.1+cpu.
| selected = weight.index_select(0, safe_targets).float() | ||
| logits = (hidden.float() * selected).sum(dim=-1) |
There was a problem hiding this comment.
Match target-logit precision to the streamed projection
For BF16/FP16 heads, _chunk_row_stats computes a low-precision matmul and only then casts its logits to FP32, whereas _target_logits keeps the unrounded FP32 dot product. The numerator and normalizer therefore use different logits: with BF16 hidden=[[3.28125]], weight=[[3.28125],[0]], and target 0, the formula produces a positive log-probability of about 0.01658 instead of about -0.00002145. This can make SFT cross-entropy negative and biases PPO ratios against the unchanged materialized scorer. Use identical projection/output precision for target logits and the log-sum-exp.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79b64b7eea57570821f1440b254d3170967e0a7b. _target_logits now performs its dot product with the same projection/output dtype as _chunk_row_stats and promotes only afterward, so the numerator and denominator consume identically rounded logits.
Added test_low_precision_target_logits_match_streamed_projection, parametrized over BF16 and FP16 and using the supplied 3.28125 example. Both cases failed before the fix (including the positive BF16 log-probability) and now match materialized log-softmax on CPU.
| rows = flat_hidden.index_select(0, positions) | ||
| return cut_cross_entropy_logprobs(rows, weight, labels, logit_softcap=logit_softcap) |
There was a problem hiding this comment.
Preserve the FP32 contract of specialized LM heads
When CCE is enabled for Phi4MM with BF16 tied weights, this call bypasses Phi4MMLMHead.forward, which explicitly disables autocast and projects both inputs and weight in FP32 (areno/models/phi4mm/model.py:870-878). The existing deferred scorer detects this output dtype and casts the head parameters once, but the new path uses the raw BF16 weight for every denominator projection. Consequently, enabling the memory optimization silently changes this model's selected scores and tied-weight gradients instead of preserving its FP32 projection contract. Detect the effective projection dtype or reject heads whose forward semantics cannot be reproduced.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79b64b7eea57570821f1440b254d3170967e0a7b. BF16/FP16 heads now receive the same output-dtype probe as the deferred scorer. For an FP32-projecting head, CCE casts the weight once for the whole autograd graph, uses FP32 hidden rows, and runs with autocast disabled, preserving the projection contract and FP32 gradient accumulation before the cast back to the tied parameter.
Added test_packed_head_cut_cross_entropy_preserves_specialized_fp32_projection, which runs under CPU autocast and compares selected scores plus hidden/weight gradients against the materialized scorer. It failed before the fix and passes with torch 2.7.1+cpu; no CUDA validation was performed.
|
Registered in #588 claim. Adds opt-in |
|
Scope/verification note (self-review against the issue's acceptance criteria):
I'd rather flag this explicitly than have the PR look complete. Options, whichever you prefer: (a) keep this PR as the code+CPU-tests deliverable and I add the CUDA report once I can borrow a GPU runner, (b) point me at a repo CI/self-hosted GPU workflow I can trigger, or (c) hold the PR until the CUDA report is attached. |
|
Correction to my previous comment: I tried switching the base to To target |
|
Addressed all three projection-precision findings in
TestsUsing
The unfiltered combined related run reached |
Closes #588.
What
Adds an opt-in cut cross-entropy (CCE) path so training can score targets from
hidden_states + lm_head.weight + target_idswithout ever materializing a[tokens, vocab]logits block.areno/accel/cut_cross_entropy.py(new)cut_cross_entropy(hidden_states, weight, target_ids, ignore_index=-100, reduction="mean")— math-equivalent to
F.cross_entropy(hidden @ weight.T, targets).cut_cross_entropy_logprobs(...)— the selected-logprob form AReno's lossfunctions actually consume.
(max, sum-of-exp)pair, and dropped; backward recomputes chunks viaactivation checkpointing instead of retaining them. Peak activation memory is
O(rows * vocab_chunk)instead ofO(rows * vocab).(bf16/fp16 safe). Supports
logit_softcap(Gemma-style),ignore_index, andreduction in {none,sum,mean}.label_smoothing != 0raises instead ofmis-scoring; an all-
ignore_indexbatch returns0instead ofnan.RuntimeConfig.cut_cross_entropy(defaultFalse), forwarded fromTrainerConfig.cut_cross_entropy/--cut-cross-entropy, consumed inTrainingManager._train_stepon the deferred LM-head path(
packed_next_token_logprobs_from_hidden). Default behaviour is untouched;a test asserts the legacy path never calls into the new module.
Tests
python -m pytest tests/test_cut_cross_entropy.py -q→ 32 passed, 2 skipped(skips: CUDA peak-memory assertion + the config test on hosts without a native
backend).
Covered:
F.cross_entropyequivalence (fp32atol/rtol=1e-5) for batch=1, singletoken, bf16/fp16 (fp32 accumulation,
rtol=1e-2), vocab not divisible by thechunk,
ignore_index, all-ignored, softcap, gradient equality (hidden + weight),duplicate targets, error paths, a saved-tensor bound proving no
[rows, vocab]block is retained, and deferred-head integration (forward + gradients) with the
flag on/off.
Baseline (no GPU on this machine)
examples/cce/README.mdrecords the CPU baseline and a GPU TODO table. On CPU(rows=2048, hidden=512, vocab=65536, chunk=4096, fp32): loss agreement
7e-6,peak RSS growth 1285 MB → 107 MB, at ~1.7x wall time — expected, since the
pure-PyTorch reference trades a single BLAS call for a chunked loop; the memory
payoff is the point and a fused CUDA kernel is the natural follow-up (tracked in
the README, not written here because this host has no CUDA to validate it).
Known limits (explicit, not silent)
tp_size=1; otherlayouts warn once and keep materialized logits.
tp_size>1) raisesNotImplementedError— it needs across-rank log-sum-exp and a custom backward.
label_smoothing.If maintainers prefer this to land on
oss-camp2026instead ofmain, say so andI'll retarget immediately.