feat(higgs-audio): add checkpoint loading and validation foundation - #864
feat(higgs-audio): add checkpoint loading and validation foundation#864ywh555hhh wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 547bab9dca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| anyhow::ensure!(!prompt.is_empty(), "prompt must not be empty"); | ||
| let start_position = kv_view.seq_len() - prompt.len(); | ||
| PrefillPagedPlan::from_raw_batch_with_cta_tile_q( | ||
| &self.ctx, |
There was a problem hiding this comment.
Move the single-prompt plan helper onto Qwen3Model
Because prefill_plan_for_single_prompt was added inside impl PrefillBuffers, self is a scratch-buffer object with only HiddenStates fields; it has no ctx, config, or local_* members. Any build of the default pegainfer-qwen3 crate will type-check this body and fail before the Higgs runtime can be built, so this helper needs to live on Qwen3Model or receive the needed context explicitly.
Useful? React with 👍 / 👎.
Signed-off-by: teapot1de <1916647616@qq.com>
e9800c5 to
3ad931a
Compare
Signed-off-by: teapot1de <1916647616@qq.com>
3ad931a to
9a39f6c
Compare
|
Small self-review note before detailed maintainer review: I realized this PR is broader than a pure
The intent was to reuse the existing Qwen3 backbone and avoid duplicating loading/forward logic inside the Higgs-Audio crate. That said, I recognize these are public/shared-layer changes and may be too much for one PR. I’m going to double-check whether each shared change is truly necessary and whether this should be split into smaller PRs before asking for full review. If maintainers prefer, I can split it into: Higgs crate/tools/docs, weight-loader alias support, Qwen3 diagnostics, and RMSNorm rounding as separate PRs. CI is currently passing, but I do not want to hide the scope concern. |
|
I want to discuss one design tradeoff in this PR before asking for deeper review. This PR no longer strictly matches the original guardrail in #408, where I said the Higgs-Audio bring-up would avoid shared crate modifications. During implementation, I found that validating against the real Qwen3-backed runtime path creates a tradeoff:
I chose the second path in this PR, but I realize this is exactly the kind of shared-layer complexity that maintainers may not want bundled into an initial Higgs-Audio PR. This also relates to the earlier guidance in #203: Higgs should be one model / one crate, and audio-specific concepts should not leak into shared layers. I think the current question is whether these changes are acceptable as general reusable seams, or whether I should split them out / keep the initial Higgs PR more isolated. Concretely, I would like maintainer input on the first tradeoff: Should Higgs-Audio reuse the existing Qwen3 loading/prefill path via small shared seams, or should the initial implementation duplicate/adapt Qwen3-like logic inside I can split this PR if the current shape is too broad. |
Signed-off-by: teapot1de <1916647616@qq.com>
Signed-off-by: teapot1de <1916647616@qq.com>
9a39f6c to
771250c
Compare
|
Follow-up after the RMSNorm rounding ablation: I removed the So the foundation PR is now narrower: it keeps the Higgs crate/tools/docs, tensor-name alias support, and Qwen3 diagnostic prefill surface, but no longer changes shared kernel code. If the rounding behavior is worth pursuing later, I will treat it as a separate Qwen3 numeric-parity discussion rather than coupling it to this Higgs-Audio foundation PR. |
|
Thanks for pushing this checkpoint-loading path forward. The tensor-name alias idea and the checkpoint-backed one-step parity gate both look useful, and I think there is a good minimal foundation inside this PR. Would you be open to reducing the scope before we review the implementation in detail? The current diff is about 10k lines across 48 files, and it combines several different concerns:
A smaller shape would be much easier to validate and maintain:
In particular, One wording detail: the committed reference path uses a Transformers Qwen3 backbone plus the SGLang tokenizer/head semantics, not the full SGLang serving runtime, and the current Rust path stops after one prefill/audio-head prediction. Describing it as a one-step compatibility foundation would set the right expectation until delay-pattern decode, codec/vocoder, and serving are added. |
|
Thanks for the detailed review. I understand the concern now. You're right that this PR has grown too broad: it mixes the generic loader alias work, Higgs-specific checkpoint/config validation, Qwen3 diagnostics, and several readiness/debugging helpers. I'm happy to narrow the scope and make the follow-up work easier to review. I'll convert this PR to draft for now, then split out the generic tensor-name alias loader into a smaller PR with a real safetensors loading test. After that, I'll reduce this PR to the minimal Higgs-Audio foundation: config/manifest validation, name mapping, fused audio head loading, a narrow final-hidden hook, and the checkpoint-backed one-step golden/parity gate. Thanks again for the clear direction. |
Summary
This PR adds the first Higgs-Audio integration foundation for PegaInfer.
The scope is intentionally limited to the checkpoint-backed validation path: parse Higgs-Audio config and checkpoint metadata, map Higgs checkpoint tensor names into the existing Qwen3 loader, run Qwen3 text-backbone prefill, run the audio-head projection validation path, and add golden-trace tools for debugging hidden-state drift.
This is not full Higgs-Audio generation yet. Delay-pattern decode, multi-codebook autoregressive audio decode, codec/vocoder integration, and waveform output are left for follow-up PRs.
Why this shape
Higgs-Audio reuses a Qwen3-style text backbone, but its checkpoint layout is not a drop-in Qwen3 checkpoint. To avoid materializing a renamed multi-GB checkpoint copy, this PR adds a tensor-name alias path in the weight loader. The Qwen3 config view stays small and explicit, while the original Higgs checkpoint remains the weight source.
I have been calling this the first integration layer during bring-up: it proves that checkpoint/config loading, Qwen3 backbone prefill, audio-head projection, and trace-based validation can work before expanding into full audio generation.
The Qwen3 diagnostic prefill hooks added here are for model bring-up and golden-trace validation. They are kept separate from the normal serving scheduler path.
What changed
pegainfer-higgs-audiowith Higgs config, manifest, load-plan, and validation utilities.docs/models/higgs-audio/a-layer-validation.md.Validation
Local checks run:
cargo fmt --check -p pegainfer-core -p pegainfer-qwen3 -p pegainfer-higgs-audio cargo check -p pegainfer-higgs-audio --bins python3 -m py_compile tools/accuracy/*.py tools/higgs/check_higgs_gate_summary.py4090 validation evidence from the checkpoint-backed validation path:
Strict elementwise parity is still diagnostic-only, but the semantic gate passes.
I also ran an RMSNorm rounding ablation. The HF/Qwen3-style fused-add-RMSNorm variant slightly improves strict trace parity, but it is not required for the current Higgs-Audio one-step semantic gate. To reduce shared-layer scope, this PR no longer changes
pegainfer-kernels; the rounding question can be discussed separately as a Qwen3 numeric-parity topic if needed.Known limitations
Maintainer question
Is the tensor-name alias loader an acceptable way to reuse the original Higgs checkpoint layout, instead of materializing a renamed Qwen3-body checkpoint copy?