fix(dmd2): restore multistep backward simulation - #1627
Closed
primorLee wants to merge 1 commit into
Closed
Conversation
primorLee
marked this pull request as ready for review
August 23, 2026 17:57
Collaborator
|
Thanks for this PR — the implementation and tests are both very thorough. We ran an actual training comparison on the FLUX.2-klein-base-4B DMD2 configuration, but we did not observe a clear quality improvement from the backward simulation version compared with the current implementation. In addition, our DMD2 implementation follows FastGen, whose multi-step training path likewise applies forward noising to real latents and then trains the selected student step, rather than using the student's preceding steps for backward simulation. For these reasons, we will not be merging this PR for now. Thanks again for the contribution and for the discussion! |
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.
Summary
torch.no_grad(), using stochastic re-noising for SDE sampling or the FlowMatch Euler update for ODE samplingWhy
For
dmd2_student_sample_steps > 1,_generate_student_datacurrently constructs the selected-step input as:and invokes the student only once. Consequently, step k is trained on a forward-noised real latent instead of the distribution produced by student steps
0..k-1at inference. Thedmd2_student_sample_typeoption is also ignored.DMD2 introduces backward simulation specifically to remove this multistep training/inference input mismatch. Its official implementation samples one step, rolls preceding student steps forward without gradients, and trains the selected step on the resulting sample:
The new SDE path re-noises each preceding x0 prediction at the next student sigma. The ODE path follows the repository's FlowMatch scheduler equation:
The one-step path remains a single student prediction from schedule-scaled noise.
Validation
The same 3 tests pass in a clean Linux Python 3.10 / CPU PyTorch container (
Ran 3 tests in 0.144s). Syntax compilation and targeted Ruff correctness checks also pass.A baseline differential check against
origin/mainreproduces the bug: with a fixed RNG seed and selected step, changing onlyreal_datafrom 0 to 100 changes the generated output by66.699997. With this patch, the generated rollout and selected-step input are identical because real data supplies only shape/device/dtype.Scope / limitation
This is a code-level correction of the DMD2 sampling distribution. I do not have an A100 end-to-end rerun of the FLUX.2 Klein 4B recipe, so I am opening this as a draft and am not claiming that it resolves every source of the loss oscillation reported in #1617. A before/after training-curve and sample-quality rerun is still needed.
Addresses #1617.