Skip to content

Validate DDPM generator list length against batch size - #14740

Open
X-mo7 wants to merge 1 commit into
huggingface:mainfrom
X-mo7:fix/ddpm-generator-list-validation
Open

Validate DDPM generator list length against batch size#14740
X-mo7 wants to merge 1 commit into
huggingface:mainfrom
X-mo7:fix/ddpm-generator-list-validation

Conversation

@X-mo7

@X-mo7 X-mo7 commented Sep 8, 2026

Copy link
Copy Markdown

What does this PR do?

This PR adds validation in DDPMPipeline to ensure that when a list of torch.Generators is provided, its length matches the requested batch_size. Before, DDPMPipeline could accept mismatched generator lists (they would silently succeed or fail later with an error depending on the list length). The new validation follows the existing behavior in the sibling DDIMPipeline and raises a clear ValueError before noise generation. A focused regression test was added to cover this case. It addresses Issue 2 in #13649.

Tests:
.venv/bin/pytest tests/pipelines/ddpm/test_ddpm.py::TestDDPMPipeline::test_generator_list_batch_size_mismatch_raises -q

1 passed

.venv/bin/pytest tests/pipelines/ddpm/test_ddpm.py -q

15 passed, 23 skipped

Formatting/style check:
PATH=.venv/bin:$PATH make modified_only_fixup

All checks passed; 2 files left unchanged

Help from AI:
Codex was used to assist with investigating the issue, implementing the change, and reviewing the final diff.

Final self-review findings:

  • The DDPM validation matches the existing DDIM validation in wording and structure
  • The validation is placed before the initial randn_tensor(...) call
  • No shared helper or unrelated refactor is needed
  • The regression test reuses the existing DDPM test helpers and covers the reported mismatch
  • No unrelated changes or outstanding self-review findings remain

Before submitting

Who can review?

cc @yiyixuxu @dg845

@github-actions github-actions Bot added size/S PR with diff < 50 LOC tests pipelines and removed size/S PR with diff < 50 LOC labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Hi @X-mo7, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice.

Once the PR links an issue (or gets the no-issue-needed label), you can ignore this message — it stays here as a comment, but it no longer applies.

@X-mo7

X-mo7 commented Sep 9, 2026

Copy link
Copy Markdown
Author

#13649 tracks multiple separate findings. This PR specifically addresses Issue 2 only, so I avoided using Fixes #13649 since that would close the whole issue while other findings remain open. I'm happy to adjust the tracking if a maintainer prefers a different approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant