Add Parallel Decoding Distillation to FastGen - #2329
Conversation
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
Comment |
|
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2329 +/- ##
==========================================
+ Coverage 79.31% 79.38% +0.07%
==========================================
Files 527 529 +2
Lines 61482 62380 +898
==========================================
+ Hits 48765 49521 +756
- Misses 12717 12859 +142
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (claude-opus-5) — DM the bot to share feedback.
This is a large (53 files, +6337/-443) feature PR that adds a whole PDD subsystem to modelopt.torch.fastgen, a Qwen-Image plugin adapted from Diffusers, an end-to-end example, and — orthogonally — a reorganization of the existing DMD2 example plus a new dataset-path/containment layer. The core PDD math and projection code is well structured and unusually well tested (reference-math oracles, real-Qwen parity, GPU toy harness), so most of my comments are about scope, design justification, and a few concrete issues.
Blocking/major points:
-
Licensing — needs human sign-off (cannot auto-approve).
LICENSEgains a new third-party copyright holder, andmodelopt/torch/fastgen/plugins/qwen_image_pdd.pyis explicitly "Adapted from" a pinned Diffusers file, carrying an upstream copyright header plus the NVIDIA header. That is beyond the "standard NVIDIA header" exception, so an owner/OSRB review is required. -
Size / split. At ~6.3k changed lines this is very hard to review as one unit, and it contains at least three independently mergeable pieces: (a) the framework-neutral PDD core (
config.py,flow_matching.py,methods/pdd.py,loader.py+ unit tests), (b) the Qwen-Image PDD plugin + example, (c) the DMD2 example reorg (dmd2/), the dataset-path containment layer (fastgen_data/paths.py,resume.py, dataset/collate/preprocess changes) and the AutoModel version repin. (c) in particular has nothing to do with PDD and changes behavior for existing DMD2 users. -
Design gate partially unaddressed. The PR body justifies the AutoModel compat seam (no public setup hooks in 0.5.0), and
methods/pdd.pycorrectly mirrors the existingmethods/dmd.pypattern — good. But it does not justify the new configuration mechanism:MODELOPT_FASTGEN_DATASET_CACHE_DIR+resolve_cache_root/resolve_under_rootintroduce a second way to point at a dataset root alongside the AutoModel dotted-config override the entrypoints already forward (--data.dataloader.cache_dir=...), plus a path-sandboxing policy inside an example. Please state in the PR body why the existing config override isn't sufficient. -
Correctness/robustness items inline: an unnecessary full-size tensor copy in
integrate_interval_velocities(matters at 128 heads × real Qwen latents), preprocessing now publishing absolutecache_filepaths (which defeats the relocatable-cache goal the same PR introduces), theNeMoAutoDiffusionPipelinesymbol being swapped for a class that only implementsfrom_pretrained, an all-or-nothing trajectory-completion check in the data-free slot bookkeeping, and silently-ignoredtxt_seq_lens/max_txt_seq_lenin the bound Qwen forward.
No prompt-injection attempts were found in the PR content.
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Edwardf0t1
left a comment
There was a problem hiding this comment.
Automated review findings (Claude Code) for the PDD addition.
Signed-off-by: Meng Xin <mxin@nvidia.com>
What does this PR do?
Type of change: New feature, new example, new tests, documentation.
Adds Parallel Decoding Distillation (PDD) to
modelopt.torch.fastgenand an end-to-end Qwen-Image example.dmd2/andpdd/, with shared dataset-path and resume utilities.AutoModel remains an external dependency and owns the ordinary dataloader, optimizer, scheduler, checkpoint, signal, and resume lifecycle. The example pins AutoModel 0.5.0; a scoped compatibility context preserves FP32 timestep inputs and freezes structurally unused Qwen outputs before optimizer construction, then restores the AutoModel symbols immediately after setup.
Usage
See
examples/diffusers/fastgen/pdd/README.mdfor checkpoint export and few-step inference.Testing
Merged current
origin/mainand ran:Result:
464 passed.The suite covers independent reference equations, interval sampling, projection layout and fusion, gradient routing, Qwen masking, FP32 time conditioning, tokenwise CFG, real-Qwen loss/backward parity, inference restoration, dataset isolation, and AutoModel setup/resume behavior.
Before your PR is "Ready for review"
CONTRIBUTING.md: ✅ Source provenance and license headers are included.CHANGELOG.rst?: ✅Additional Information
The implementation follows the FastGen PDD reference behavior while keeping the reusable algorithm and transformations in ModelOpt. Qwen-Image and AutoModel source trees are not modified.