Repository navigation
fix(loss): make LigerFusedLinearCrossEntropyLoss follow the CrossEntropyLoss contract - #335
Merged
tastelikefeet merged 1 commit intoOct 26, 2026
Conversation
…opyLoss contract - reduction='mean': the fused path returned num_tokens = token count, so calculate_loss divided the already averaged loss by the token count again. Return num_tokens=0 like CrossEntropyLoss. - micro-batching / dynamic batching raised NotImplementedError because the loss had no micro_batch_scale; use CrossEntropyLoss's token-weighted scale. - the materialised fallback (used after the fused kernel fails) ignored softcap, so its loss differed from the fused kernel's for softcapped models. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tastelikefeet
approved these changes
Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR type
PR information
LigerFusedLinearCrossEntropyLossis documented as a drop-in forCrossEntropyLoss, but it differs from it in three places.reduction='mean'normalizes twice. In the fused path the loss always returnednum_tokens = #non-ignored tokens.calculate_losstreats a non-zeronum_tokensas "sum loss, divide the gradient by the token count", which is theCrossEntropyLoss(reduction='sum')contract.CrossEntropyLoss(reduction='mean')returnsnum_tokens=0. So withreduction='mean', an option the docstring offers, the already averaged loss was divided by the token count again, and the effective learning rate shrank with batch size. The fused path now returnsnum_tokens=0for'mean'.'sum'(what the cookbooks use) is unchanged.NotImplementedError. The class had nomicro_batch_scale, soforward_backward(..., micro_batch_size=...)and dynamic batching hit the base-classNotImplementedError. It now uses the same token-weighted scale asCrossEntropyLoss(1.0 for'sum').softcap. After the fused kernel fails,_materialise_cerecomputes logits withF.linearbut did not apply the softcap that was passed to the fused kernel. For softcapped models the fallback therefore computed a different loss, despite the "same numerics" note. It now appliessoftcap * tanh(logits / softcap).Experiment results
liger_kernelneeds CUDA/NPU, so the new CPU tests swap the kernel for a pure-torch module with the same semantics (F.cross_entropy(F.linear(...)), with the optional softcap). They cover:test_fused_path_num_tokens_matches_cross_entropy[mean|sum]: loss andnum_tokensmatchCrossEntropyLoss;test_micro_batch_scale_matches_cross_entropy[mean|sum];test_materialised_fallback_applies_softcap.Four of the five fail on
main. Thesum/num_tokenscase already passed. All five pass with this change.The device tests (
requires_liger/requires_device) were not run here, because I have no GPU. The fused-kernel call itself is unchanged.🤖 Generated with Claude Code