QVAC-23944 audiogen: add ACE-Step Multi-Track (lego) task with base-model CFG/APG sampling - #164
QVAC-23944 audiogen: add ACE-Step Multi-Track (lego) task with base-model CFG/APG sampling#164freddy311082 wants to merge 5 commits into
Conversation
…odel CFG/APG sampling - Add classifier-free guidance to the DiT sampler via APG (Adaptive Projected Guidance): per-step unconditional forward with the null condition embedding, momentum -0.75, per-channel norm clip 2.5 and orthogonal projection, matching the official base/sft path. - New GenerateParams: guidance_scale (0 = auto: turbo 1.0, base 7.0) and track (lego target layer). - New task_type "lego": generates a named instrument layer over source_audio and returns the isolated stem. Requires a base/sft DiT (turbo rejected), validates the 12 official track names, skips the LM, and locks output length to the source. - Apply Haar DCW on turbo DiTs only (official base preset disables it). - Default the lyric-stream language to "unknown" for lego, matching the reference conditioning for stem tasks. - music-cli: --track and --guidance flags plus request JSON fields. - Tests: lego task validation/plan units, APG golden-value unit, lego integration scenario with context-dump verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review StatusCurrent Status: ❌ PENDING Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member. |
GustavoA1604
left a comment
There was a problem hiding this comment.
[critical] engines/audiogen/src/acestep/engine.cpp — line 963 (if (is_lego_task(task.type) && config.is_turbo))
This permits LEGO generation with SFT weights, but ACE-Step defines LEGO as a Base-only task; SFT supports CFG but not the Base-exclusive extract/lego/complete tasks. Require an actual Base model identity rather than merely !is_turbo, and add an SFT rejection test.
[medium] engines/audiogen/test/test_acestep_integration.cpp — line 466 (CHECK(result.pcm.size() >= params.source_audio.size());)
LEGO promises a stem aligned to the source, but source latent frames are rounded to the DiT patch size and the decoded PCM is never trimmed. Arbitrary WAV lengths can therefore produce a longer stem that cannot be mixed sample-for-sample. Preserve the original source sample count, trim the result to it, and assert equality here.
[medium] engines/audiogen/src/acestep/engine.cpp — line 1048 (if (params.guidance_scale > 0.0f) return params.guidance_scale;)
An explicit guidance_scale > 1 enables APG on Turbo even though Turbo is guidance-distilled and CFG is unsupported. Force guidance to 1.0 for Turbo or reject such an override; otherwise --guidance 7 can run an invalid second forward and degrade output.
[medium] engines/audiogen/test/test_acestep_integration.cpp — line 468 (catch (const std::invalid_argument & error))
The integration test treats a Turbo fixture as a successful skip, so the green PR checks never execute LEGO generation. Add a Base-model CI lane
[medium][coding-standards] engines/audiogen/src/acestep/engine.cpp — line 1047 (static float resolve_guidance_scale(...))
The new guidance defaults, explicit override, and model-specific DCW gate have no unit coverage; the APG golden test does not exercise this policy. Add tests for Turbo/Base automatic guidance, explicit overrides, and DCW being enabled only for Turbo.
[low] engines/audiogen/README.md — line 92 (DiT CFG and APG ... are deferred)
This is now false, and the README also omits LEGO usage, --track, --guidance, and the new request fields. Update both this engine README and the repository root README as required for new public functionality.
[low][coding-standards] engines/audiogen/src/acestep/dit_ggml.cpp — line 909 (for (size_t i = 0; i < diff.size(); i++))
dit_apg_guide contains three separate loops/actions, contrary to the team rule that each loop be isolated in its own function. Extract the difference calculation, per-batch projection, and guided-update application into named helpers.
[low][coding-standards] engines/audiogen/src/acestep/dit_ggml.cpp — line 833 (// APG (Adaptive Projected Guidance)...)
This newly added explanatory block narrates implementation details, which the team standard prohibits. The helper and constant names already express the behavior; remove the narration or encode any missing intent in names.
- Require an actual base DiT for lego: sft is detected from the GGUF general.name and rejected alongside turbo, with unit coverage. - Trim the lego stem to the source sample count so it mixes sample-for-sample; the integration scenario now asserts equality. - Clamp guidance overrides to 1.0 on turbo (guidance-distilled) and cover the guidance/DCW policy with unit tests. - Add an opt-in base-model integration lane via AUDIOGEN_TEST_BASE_MODELS_DIR where lego must execute for real. - Split dit_apg_guide into single-purpose helpers and drop the narrative comment block. - Document the lego task, --track/--guidance, per-variant guidance defaults and base-only stem support in both READMEs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the full review in 1aabecc — thanks for the sharp catches, especially the sft gap:
894/894 unit checks pass; E2E re-verified locally: turbo rejection message, base run with guidance 7, and stem length equal to the source. |
Extract is_sft_model_name next to DitConfig and cover the checkpoint naming matrix (sft, xl-sft, base, xl-base, turbo, empty). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the output/ audio, log, and tensor-dump files that a broad git add swept into the review-fixes commit, and gitignore the directory so local test artifacts cannot be committed again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Adds the ACE-Step Multi-Track (lego) task to the audiogen engine: given a source song and a target track name, the DiT generates a new isolated instrument layer that follows the source (tempo, key, groove), returned as a separate stem for mixing.
This required unlocking the base/sft DiT sampling path, which was deferred until now: classifier-free guidance via APG (Adaptive Projected Guidance) with the null condition embedding, momentum -0.75, per-channel norm clipping at 2.5, and orthogonal projection — matching the official implementation.
Changes
dit_apg_guidecombine;guidance_scaleandnull_cond_embsampling params.task_type: "lego"with the official per-track instruction (uppercase track name), base/sft enforcement (turbo rejected with a clear error), guidance auto-resolution (turbo 1.0 / base 7.0), Haar DCW applied on turbo only (official base preset disables it), and lyric-stream language defaulting to "unknown" for lego (reference parity for stem tasks).GenerateParams::guidance_scaleandGenerateParams::track; 12 official track names validated.--track,--guidance, and request JSON fields.Validation
🤖 Generated with Claude Code