Skip to content

Fix diffusion training and sampling issues#67

Merged
njhoffman11 merged 1 commit into
mainfrom
codex/address-diffusion-issues
May 18, 2026
Merged

Fix diffusion training and sampling issues#67
njhoffman11 merged 1 commit into
mainfrom
codex/address-diffusion-issues

Conversation

@SoheylM

@SoheylM SoheylM commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

I fixed the diffusion issues reported in #64, #65, and #66.

  • I fixed the 1D diffusion training path so designs are normalized before being reshaped and passed into GaussianDiffusion1D.
  • I wired the 1D auto_norm argument into both training and evaluation so restored runs use the saved normalization behavior.
  • I changed the conditional 2D diffusion default from 250 to 1000 timesteps. With the existing linear beta schedule, 250 steps left sqrt(alpha_bar_T) ~= 0.282, while 1000 steps brings it to ~0.00635, which better matches sampling from Gaussian noise.
  • I fixed DiffusionSampler.diffusion_step_sample to use fresh Gaussian noise for the posterior variance term instead of reusing the predicted noise, and to avoid adding noise at t=0.
  • I added focused regression tests for the 1D normalization path, the 2D schedule sanity check, and the sampler variance-noise behavior.

Validation

I validated this in the engibench Conda environment after running the documented EngiOpt install flow (pip install -e .).

  • python -m pip check passed with no broken requirements.
  • python -m pytest -q passed (3 passed).
  • ruff check passed on the changed diffusion files and tests.
  • ruff format --check passed on the changed diffusion files and tests.
  • python -m compileall engiopt/diffusion_1d engiopt/diffusion_2d_cond tests passed.
  • I ran a 1D smoke check on real EngiBench airfoil data: one forward loss, backward pass, optimizer step, and sample all completed with finite values and nonzero gradients.
  • I ran a 2D conditional smoke check on real EngiBench beams2d data: 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 tests still 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

@njhoffman11

Copy link
Copy Markdown
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

@njhoffman11
njhoffman11 merged commit c449f76 into main May 18, 2026
3 checks passed
@njhoffman11
njhoffman11 deleted the codex/address-diffusion-issues branch May 18, 2026 14:10
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants