Skip to content

[tests] Fix controlnet tests#13736

Merged
sayakpaul merged 8 commits into
mainfrom
fix-controlnet-tests
May 15, 2026
Merged

[tests] Fix controlnet tests#13736
sayakpaul merged 8 commits into
mainfrom
fix-controlnet-tests

Conversation

@sayakpaul
Copy link
Copy Markdown
Member

@github-actions github-actions Bot added tests size/S PR with diff < 50 LOC labels May 13, 2026
@sayakpaul
Copy link
Copy Markdown
Member Author

@claude-2-serge could you perform a review?

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤗 Serge says:

Straightforward fix switching four controlnet test files from float16 to float32 for dummy/fast tests, with updated expected output slices. The changes are consistent and correct.

Minor issue

  • In test_controlnet_flux.py, there is one remaining float16 at line 201 inside test_flux_image_output_shape that was not updated. The pipe in that test runs in float32 (no dtype override), but the control_image tensor is still created as float16. This likely doesn't cause a failure since the test only checks output shape, but it's inconsistent with the rest of the fix and could mask a dtype-mismatch issue.

5 LLM turns · 7 tool calls · 30.6s · 48948 in / 1398 out tokens

generator=generator,
device=torch.device(device),
dtype=torch.float16,
dtype=torch.float32,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: there's another dtype=torch.float16 at line 201 in test_flux_image_output_shape that wasn't updated. The pipe in that test defaults to float32, so the control_image will be float16 while the rest of the pipeline is float32. It probably doesn't break anything since that test only checks output shape, but for consistency it should likely be torch.float32 as well.

@sayakpaul sayakpaul requested a review from DN6 May 13, 2026 09:51
@sayakpaul sayakpaul requested a review from dg845 May 15, 2026 00:55
Copy link
Copy Markdown
Collaborator

@dg845 dg845 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The change from FP16 to FP32 looks good to me, and the tests look fixed as well.

But why does switching to FP32 fix the underlying issue(s) that caused the tests to fail in the first place? I see that full inference in the slow tests uses BF16 (Flux) or FP16 (Hunyuan, SD3); if there really is an underlying issue affecting FP16/BF16 ControlNet inference specifically, it seems like the (fast) tests won't catch it.

@sayakpaul
Copy link
Copy Markdown
Member Author

They are not fixing the assertion problem (the assertion slices are being changed). It's just a safeguard to avoid numerical problems in CPUs where our fast tests run.

@sayakpaul sayakpaul merged commit e2dae06 into main May 15, 2026
11 of 14 checks passed
@sayakpaul sayakpaul deleted the fix-controlnet-tests branch May 15, 2026 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants