Repository navigation
[diffusion] warmup: default to model sampling resolution (declare Z-Image default) - #29519
Conversation
…-Image default Server-based image warmup shrank to an area cap (SERVER_WARMUP_IMAGE_MAX_AREA, 768x768), leaving a ~0.1s first-request cold-start when the real request is larger (e.g. 1024x1024 paid first-shape kernel autotuning). Default to the model's sampling_defaults resolution so warmup specializes kernels for the likely real shape — verified on H100: flux1/ideogram warm at 1024 and match the client-warmup baseline (4.70s/5.21s vs 4.68s/5.19s) instead of ~0.1s slower. Video keeps the area/frame caps (a full-resolution video warmup is far costlier). Z-Image declared no default resolution (it accepts arbitrary /16 resolutions), so it fell back to the area cap; declare its official default 1024x1024 (supported_resolutions stays None = all allowed) so it benefits too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the default resolution for Z-Image sampling parameters to 1024x1024 and adjusts the warmup request builder to use the model's default resolution for image generation tasks during server-based warmup, avoiding cold-start autotuning overhead. Feedback suggests evaluating the image generation check lazily to prevent potential attribute errors when server arguments are partially initialized or mocked in tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| is_image_gen = server_args.pipeline_config.task_type.is_image_gen() | ||
| if ( | ||
| width is not None | ||
| and height is not None | ||
| and (not server_based_warmup or is_image_gen) | ||
| ): |
There was a problem hiding this comment.
The is_image_gen variable is evaluated unconditionally, even when server_based_warmup is False. When server_based_warmup is False, the condition not server_based_warmup is True, meaning is_image_gen is not needed to determine the outcome.\n\nEvaluating this unconditionally can lead to unnecessary attribute access and potential AttributeError or NoneType errors if server_args is mocked or partially initialized (e.g., in unit tests).\n\nWe can leverage Python's short-circuit evaluation to lazily evaluate is_image_gen only when server_based_warmup is True.
if (\n width is not None\n and height is not None\n and (not server_based_warmup or server_args.pipeline_config.task_type.is_image_gen())\n ):References
- Enforce defensive programming by ensuring appropriate guards exist before object property accesses, especially when inputs might be partially initialized or mocked in tests.
|
/tag-and-rerun-ci |
#29519 makes server-based image warmup use the model's default resolution (warming up at the real inference shape to remove a residual cudagraph/compile gap) instead of scaling down to a server-warmup area budget. Update the three affected unit tests to assert the model default (1024x1024) and rename them to reflect the new contract. Video warmup stays budget-limited (test_server_based_warmup_keeps_video_warmup_lightweight unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#29519 makes server image warmup use the model default resolution (1024x1024 for FLUX.2-klein), so warmup now specializes kernels for the real inference shape. This shifts per-step CPU-wall-clock timing (step0 drops as it no longer pays first-shape autotune, step2/3 rise to true steady state) WITHOUT changing total work: e2e (483 vs 434 baseline), avg_denoise (40.72 vs 39.98) and median are all within tolerance. The old denoise_step_ms baseline was internally inconsistent: its steps summed to ~27.7ms/step (110.87/4) while expected_avg_denoise_ms was 39.98, because #28123 tightened only step2 (54.97 -> 17.64) without touching avg. Update denoise_step_ms to the measured 1024-warmup values so it is self-consistent with avg (sum/4 = 40.72 ~= 39.98). No code/perf regression; e2e/avg/median baselines unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…blip) fsdp-inference's denoise step6 baseline (232.55) was depressed by #28123 ("CI: tighten perf baselines", 270.89 -> 232.55) onto a single noisy CI sample, leaving denoise_step_ms internally inconsistent (sum/n=293 vs expected_avg_denoise_ms=263.19). The ~600ms cost is a single CPU-wall-clock blip (per-step timers don't sync CUDA) that floats across the denoise loop: the baseline pinned it onto step3 (650.0, done in #28781) but at runtime it lands on step6 instead, while step3 comes back normal (~272). Both step3 and step6 are sampled indices ({0,2,3,5,6,8}), so whichever step catches the blip fails. Give step6 the same ceiling as step3 (232.55 -> 650.0) so either step can absorb the blip. avg_denoise / median / e2e baselines unchanged, so a real compute regression still trips the aggregate checks (no masking). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Motivation
Server-based image warmup defaulted to an area-capped "representative" resolution (
SERVER_WARMUP_IMAGE_MAX_AREA= 768×768). For larger real requests (e.g. 1024×1024) the first request still paid first-shape kernel autotuning — a ~0.1s residual measured on H100, even though warmup ran.Fix
_resolve_default_warmup_resolution: for image warmup, prefer the model'ssampling_defaultswidth/height (the most likely real request shape) instead of the area-capped representative, so kernels are specialized for the actual shape. Video keeps the area/frame caps (a full-resolution video warmup is far costlier). Representative selection remains the fallback when a model declares no default width/height.Z-Image declared no default resolution (it accepts arbitrary /16 resolutions, so
width/heightwere leftNone), and therefore fell back to the cap. Declare its official default 1024×1024 (supported_resolutionsstaysNone= all allowed, so other resolutions still work without spurious warnings).Verification (H100)
serve --warmupwith no explicit--warmup-resolutionsnow warms at the model default and matches the client-side-warmup baseline:Relationship
Complements the
--warmupdead-zone fix (so server-based warmup actually runs) — that fix is tracked separately. This PR is purely about which resolution the default warmup uses.🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ✅ Run #28390506011
Latest PR Test (Extra): ❌ Run #28390505790