Fix diffusion training and sampling issues#67
Merged
Conversation
Contributor
|
These seem reasonable, but I don't see how this would fix the ghosting issue you see. I would suggest trying an updated version of huggingface diffusers, to rule out issues there |
mkeeler43
added a commit
that referenced
this pull request
May 27, 2026
Propagate PR #67 changes to the image-conditioned variant: - num_timesteps 250 -> 1000 so the terminal schedule matches the Gaussian sampling prior (the behaviorally-meaningful change) - diffusion_step_sample now uses fresh noise + t_mask and infers device from the input, keeping it byte-for-byte aligned with diffusion_2d_cond (note: this method is currently unused; the live sampling path sample_timestep already had the fresh-noise fix) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AnnaDelbeke
pushed a commit
to AnnaDelbeke/EngiOpt
that referenced
this pull request
Jun 26, 2026
…issues Fix diffusion training and sampling issues
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
I fixed the diffusion issues reported in #64, #65, and #66.
GaussianDiffusion1D.auto_normargument into both training and evaluation so restored runs use the saved normalization behavior.sqrt(alpha_bar_T) ~= 0.282, while 1000 steps brings it to~0.00635, which better matches sampling from Gaussian noise.DiffusionSampler.diffusion_step_sampleto use fresh Gaussian noise for the posterior variance term instead of reusing the predicted noise, and to avoid adding noise att=0.Validation
I validated this in the
engibenchConda environment after running the documented EngiOpt install flow (pip install -e .).python -m pip checkpassed with no broken requirements.python -m pytest -qpassed (3 passed).ruff checkpassed on the changed diffusion files and tests.ruff format --checkpassed on the changed diffusion files and tests.python -m compileall engiopt/diffusion_1d engiopt/diffusion_2d_cond testspassed.airfoildata: one forward loss, backward pass, optimizer step, and sample all completed with finite values and nonzero gradients.beams2ddata: one forward loss, backward pass, optimizer step, terminal schedule check, and sampler step all completed with finite values and nonzero gradients.Notes
A full-repo
ruff check engiopt testsstill reports pre-existing import-order issues in unrelated files. I kept this PR scoped to the diffusion fixes and verified the touched files instead.Fixes #64
Fixes #65
Fixes #66