perf(eagle3): skip unused teacher projections while preserving metrics - #1094
Draft
julyanghar wants to merge 1 commit into
Draft
perf(eagle3): skip unused teacher projections while preserving metrics#1094julyanghar wants to merge 1 commit into
julyanghar wants to merge 1 commit into
Conversation
Signed-off-by: julyanghar <julyang5216@gmail.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews
🔴 Require approval from approved reviewers listWaiting for any of
This rule is failing.All pull requests must have at least one approving review from a member of the approved reviewers list before merging.
|
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.
EAGLE3 currently normalizes/projects every verifier hidden row before applying the loss mask. This draft projects only the rows required by the loss and the existing cross-depth accuracy history, then scatters into the unchanged dense targets layout. The draft backbone, draft logits/tokens, and metric definitions are unchanged.
Draft because the performance benefit is workload-dependent: contiguous prompt/response masks benefit, but dense masks and some short dispersed masks regress. This is not yet a recommendation to enable the change for every workload.
The important correctness constraint is that
loss_maskalone is insufficient.prev_correctbegins at supervised draft starts and compares start r against teacher row r+t at depth t, including masked intermediate targets. Keep the union of those positions for t in [0, ttt_steps). A three-position [1,0,1] regression test shows that zeroing masked targets alone preserves loss but changes later accuracy counts. The implementation preserves that existing behavior rather than changing metrics as part of a performance patch. With no mask or all rows needed, it retains dense projection (the latter still pays mask-analysis overhead).Validation on base
3419401a380db305ed6493ba4680ad8a3c3e0e45:CUDA_VISIBLE_DEVICES='' python -m pytest tests/integration/models/test_eagle3_target_projection.py -q -o addopts='': 106 passed. Seven built-in eager losses, five mask layouts, depths 1/3/4, packed documents; real model loss, complete output metrics, draft tokens, and parameter gradients compared against dense teacher projection. Includes the mask-only negative control.CUDA_VISIBLE_DEVICES=2 python -m pytest tests/integration/models/test_eagle3_target_projection.py tests/integration/models/test_model_forward.py -k 'eagle3 or Eagle3' -q -o addopts='': 50 passed, 100 deselected, before expanding the new loss parameterization from KL to all seven losses. Includes existing compiled CUDA EAGLE3 forward/backward and attention-backend checks.CUDA_VISIBLE_DEVICES='' python -m pytest tests/unit/models/test_metrics.py tests/unit/models/test_eagle3_attention.py -q -o addopts='': 46 passed.git diff --checkpassed for changed files.Benchmark scope: teacher norm/projection/selection/scatter only, not complete training. RTX 6000 Ada; PyTorch 2.11.0+cu130; BF16; B=1, H=1024, V=32768, TTT=7. Each arm wrapped with
torch.compile, three warmup calls, four alternating AB/BA pairs, five calls per arm, synchronized host wall timing without profiler. Compilation is outside timing. Dense zero-filled targets remain allocated. All required-row logits were bit-identical in these sampled eager BF16 checks and argmax agreed; this is not a universal bitwise-equivalence claim.The mask's future-position union explains why ~10% dispersed supervision can require ~50% of teacher rows. Nonzero/dynamic selection adds synchronization and compiled graph boundaries, so the full-mask fallback is not free. Eager function measurements also showed a short dispersed-mask slowdown (~13%). Full training throughput, distributed training, and serving/quality metrics have not been measured.
Related: #1088 replaces the metrics contract; if it lands first, the required-row rule and its tests need to be revisited. This PR targets current main and does not duplicate the metric-definition changes in #1088, #624, or #902.
Reproduce the function-only benchmark from the PR checkout
Save as a Python file and run with the desired
CUDA_VISIBLE_DEVICES. It writes timing JSON, environment/base identity, and a working-tree patch intoteacher-projection-benchmark/. The PR diff supplies the committed patch.