Skip to content

feat(eagle3): Qwen3 EAGLE-3 drafter forward pass (single-step + chain rollout) - #707

Open
scatyf3 wants to merge 1 commit into
pegainfer-project:mainfrom
scatyf3:feat/eagle3-drafter-forward
Open

scatyf3 wants to merge 1 commit into
pegainfer-project:mainfrom
scatyf3:feat/eagle3-drafter-forward

Conversation

@scatyf3

@scatyf3 scatyf3 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

This is an follow up PR for #662

What

Adds the EAGLE-3 drafter forward path for the Qwen3 line, on top of the drafter config/loading skeleton. The core change is in openinfer-qwen3/src/eagle3/forward.rs

Code

  • draft_step: one drafter decoding step
  • prefill_prompt / prefill_batched: teacher-forced prefill that builds the
    drafter KV and the boundary target feature from captured hidden states.
  • draft_chain: autoregressive chain of γ draft tokens from the
    fused target boundary hidden; v1 syncs logits to host per step for the argmax.
  • chain_round: one speculative round: init the residual stream from
    fc(boundary feature), draft γ tokens, then rewind the speculative KV so the
    round is side-effect-free except for the returned span.
  • rebuild_after_verify: after a verify step, teacher-force the accepted
    prefix back into the drafter KV and set the new boundary target feature for the next round.

Testing

  • cargo test --release -p openinfer-qwen3 --lib — reservation geometry unit
    test passes; cargo fmt/clippy clean.
  • No GPU e2e here — the forward path is exercised end-to-end by the follow-up
    scheduler PR that consumes these entry points and perform accuracy check vs hf golden gate.

xiaguan commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Could you account for the extra committed token in the capacity check? draft_chain allows start_position + k <= max_cache_len, but when all k drafts match, rebuild_after_verify commits k + 1 tokens (the current token plus k drafts). For example, start=97, k=3, max=100 passes drafting but overflows during rebuild. Please either reserve space for k + 1 or shrink k near the boundary, and add an all-accepted boundary test.

@scatyf3

scatyf3 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for catching the bug, I already fix that

@github-actions

Copy link
Copy Markdown

This pull request has been inactive for 14 days. It will be closed after another 30 days unless there is new activity.

@github-actions github-actions Bot added the stale Automatically marked after inactivity label Aug 10, 2026

@FeathBow FeathBow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @scatyf3 still working on this? Could you please rebase onto main when you get a chance? btw main has moved forward a lot, so we might need to rethink this.

@scatyf3

scatyf3 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@FeathBow yes, i'm insterested in working on this, I'll rebase it to main this weekend. I understand EAGLE is not the SOTA sd draft and it maynot has top priority and might not be merged into main

@FeathBow

FeathBow commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Oh thanks @scatyf3! Just to clarify, I reckon this is a super interesting enhancement and don't sweat too much about sota part :) but we've just merged a few refactors and crate adjust so we might need to rewire things a bit when rebasing.

@github-actions github-actions Bot removed the stale Automatically marked after inactivity label Sep 6, 2026
@scatyf3
scatyf3 force-pushed the feat/eagle3-drafter-forward branch from 19a9a2b to e6bc593 Compare September 7, 2026 02:20
scatyf3 pushed a commit to scatyf3/pegainfer that referenced this pull request Sep 7, 2026
… rollout)

Rebase of pegainfer-project#707 onto current main. The forward path itself is unchanged; only
what main's churn required:

- `openinfer-qwen3` -> `pegainfer-qwen3` and `openinfer_{core,kernels}` ->
  `pegainfer_{core,kernels}` (rename in pegainfer-project#841).
- import granularity / ordering per the rustfmt config adopted in pegainfer-project#737.
- drop the unused `EAGLE3_CHAIN_LENGTH` test import left by the capacity fix
  (19a9a2b); it failed `clippy --all-targets -D warnings`.
- typo fixes in the new doc comments.

Every op the drafter calls (`eagle3_rope_into`, `single_decode_nhd_into`,
`single_prefill_nhd_causal_into`, `fused_add_rms_norm_round_batch_into`, ...)
still exists on main with the same signature, so no logic changes were needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… rollout)

Rebase of pegainfer-project#707 onto current main. The forward path itself is unchanged; only
what main's churn required:

- `openinfer-qwen3` -> `pegainfer-qwen3` and `openinfer_{core,kernels}` ->
  `pegainfer_{core,kernels}` (rename in pegainfer-project#841).
- import granularity / ordering per the rustfmt config adopted in pegainfer-project#737.
- drop the unused `EAGLE3_CHAIN_LENGTH` test import left by the capacity fix
  (19a9a2b); it failed `clippy --all-targets -D warnings`.
- typo fixes in the new doc comments.

Every op the drafter calls (`eagle3_rope_into`, `single_decode_nhd_into`,
`single_prefill_nhd_causal_into`, `fused_add_rms_norm_round_batch_into`, ...)
still exists on main with the same signature, so no logic changes were needed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: scatyf3 <13589360+scatyf3@user.noreply.gitee.com>
@scatyf3
scatyf3 force-pushed the feat/eagle3-drafter-forward branch from e6bc593 to 657718c Compare September 7, 2026 02:22

@xiaguan xiaguan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for rebasing this and for fixing the cache-capacity boundary. I re-reviewed the exact current head, 657718c9.

Performance evidence first: this head has no runnable EAGLE path and provides no same-head A/B data. EAGLE3_CHAIN_LENGTH = 3 cites an earlier RTX 5070 Ti GSM8K result without the raw measurements or baseline, while draft_chain currently performs a full-logit device-to-host copy and stream synchronization for every draft token (pegainfer-qwen3/src/eagle3/forward.rs:663-701). We therefore cannot validate the claimed chain optimum or an end-to-end speedup from this PR.

I do not think this is ready to merge yet for three reasons:

  1. The entire forward implementation is unreachable in production. pegainfer-qwen3/src/eagle3.rs:2 suppresses dead-code diagnostics for the module, and the exports at lines 14-22 explicitly wait for a future scheduler PR. Every new request-state, scratch, prefill, draft, rebuild, and memory-reservation entry point has no caller outside this module. There is also no EAGLE CLI/load path, KV-budget integration, request ownership/drop handling, target-capture dispatch, or scheduler execution path. Green compile checks cannot establish the behavior of code that the product cannot invoke.

  2. The tests do not execute the model forward or its state transaction. The three focused EAGLE tests cover only auxiliary-layer arithmetic, reservation arithmetic, and chain capacity. The load-bearing contracts remain untested: feature/token shifting in prefill_prompt, capture alignment, KV rewind/rebuild after full match and mismatch, reduced-vocabulary mapping, and cleanup on failure. Please add a real GPU end-to-end gate against an HF/reference oracle, including lossless output equivalence and cache-edge/abort cases.

  3. Current main already has a shared speculative transaction that this implementation needs to join. speculative.rs defines the token-only DraftPlan -> DraftResult -> VerifyPlan -> VerifyResult seam, and executor/spec.rs owns verify/accept/KV commit. The remaining executor readiness, hidden-state capture, and worker-lane state are currently DFlash-specific. Please land the smallest usable opt-in vertical slice through that existing transaction (single-request greedy is a reasonable first scope, with unsupported combinations rejected at startup), including memory reservation and request cleanup. This will also show which abstractions are genuinely shared by two proposers.

Once a real caller exists, please remove fuse_input_hidden_from_context if it remains an unused test helper, and stop returning the two prefill_batched outputs that the caller does not consume (forward.rs:382-392). Then provide same-head speculative-off/EAGLE c1 and concurrent A/B results, accepted-length distribution, and host-sync evidence.

The forward code may be useful groundwork, but merging 883 lines behind allow(dead_code) would leave the repository responsible for an implementation whose correctness and performance cannot currently be exercised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants