Skip to content

Remove per-step host syncs from the Z-Image denoising loops - #14861

Open
adrianrfreedman wants to merge 1 commit into
huggingface:mainfrom
adrianrfreedman:zimage-no-host-sync-in-denoising-loop
Open

adrianrfreedman wants to merge 1 commit into
huggingface:mainfrom
adrianrfreedman:zimage-no-host-sync-in-denoising-loop

Conversation

@adrianrfreedman

Copy link
Copy Markdown

What does this PR do?

Removes the per-step device-to-host sync from the five Z-Image denoising loops that still have one. Split out of #14589 so each model family can be reviewed on its own. Part of #13401, same class of problem as #11696, #13404, #13406, #13461, and #13564.

The problem

Five Z-Image pipelines read the timestep off a device tensor inside the loop:

pipeline read
z_image/pipeline_z_image_omni.py timestep[0].item()
z_image/pipeline_z_image_controlnet.py timestep[0].item()
z_image/pipeline_z_image_controlnet_inpaint.py timestep[0].item()
z_image/pipeline_z_image_img2img.py timestep[0].item()
z_image/pipeline_z_image_inpaint.py timestep[0].item()

ZImagePipeline already solves this. It hoists _precomputed_t_norms = ((1000 - timesteps.float()) / 1000).tolist() out of the loop, guarded on cfg truncation. The other five were left behind, so this applies your existing pattern to them: same guard, same name, same expression.

Recomputing the normalisation in Python floats after tolist() is not equivalent, since the tensor arithmetic runs in the tensor's dtype, so hoisting the tensor expression is what keeps it bit-identical.

Quality

Unchanged. The normalisation is bit-identical in fp32, fp16, and bf16.

Measurements

ZImageImg2ImgPipeline, Tongyi-MAI/Z-Image-Turbo, 1024x1024, 8 steps, one L40S, each configuration run in both orders.

main this PR
sync check inside the loop fails passes
CPU blocked in syncs 1581 ms 110 ms
wall clock 4644.3 ms 4644.0 ms

No wall-clock change, eager or compiled. Expected, since the GPU does identical work and the model is GPU-bound at this size. What it buys is 1.5 s per run of unblocked CPU, which matters on a busier host, and a loop that no longer syncs at all.

I measured img2img. The other four take the identical change to a file I did benchmark.

Left alone

set_begin_index(0) is missing from omni, controlnet, and controlnet_inpaint, which is the separate sync from #11696. I have not touched it here, because img2img and inpaint can legitimately begin partway through the schedule and that deserves its own look.

Tests

tests/pipelines/z_image/: 112 passed, 39 skipped. No new test. The sync-debug test in #14589 pins the pattern, and adding one here would need the real checkpoint on a GPU runner. Say if you would rather have one.

Self-review

Ran the self-review skill over the diff. Nothing blocking.

Before submitting

Who can review?

@sayakpaul

ZImagePipeline already hoists the timestep normalisation out of the loop
with _precomputed_t_norms. The other five Z-Image pipelines still read
timestep[0].item() every step, which blocks the CPU on a device-to-host
copy. Apply the same pattern: same guard, same name, same expression, so
the normalisation stays bit-identical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

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

Labels

pipelines size/M PR with diff < 200 LOC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant