Skip to content

fix(stable_diffusion): wire SDXL qualification - #1909

Open
JiaxinD wants to merge 1 commit into
NVIDIA:mainfrom
JiaxinD:fix/sdxl-benchmark-controls
Open

JiaxinD wants to merge 1 commit into
NVIDIA:mainfrom
JiaxinD:fix/sdxl-benchmark-controls

Conversation

@JiaxinD

@JiaxinD JiaxinD commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Background

The existing sdxl-turbo qualification profile fails request preflight: the resolved request leaves num_steps and cfg_scale at their default sentinels. It also lacks the family latent adapter, so generic replay expects a transformer config from this UNet checkpoint. The existing family reference always loads SD1.5/DDIM.

Exit Criteria

  • Resolve explicit one-step, zero-guidance SDXL requests and family-owned latent replay.
  • Select the checkpoint's SDXL/Euler reference while preserving SD1.5/DDIM behavior.
  • Preserve all accuracy/performance gates. GPU qualification remains separate.

Implementation

  • Add families/stable_diffusion/tests/aiperf/sdxl-turbo.yaml with explicit controls and reference/replay adapters.
  • Select the official SDXL pipeline and Euler ancestral scheduler from the pinned pipeline config. Keep SD1.5 safety options and DDIM; disable the SDXL watermarker consistently with the existing E2E reference.
  • Add dispatch/control regressions for both pipelines in eager and compile modes, including zero guidance, revision forwarding and caller latents.
  • No shared application/core edits, dependency changes, native topology, public API/ABI or bundle-format changes. Existing bundles need no rebuild; restart reference processes to load the updated adapter.

Change categories

  • Model or runtime behavior
  • Public API
  • ABI
  • Bundle or artifact format
  • Dependencies
  • Documentation only
  • CI or developer tooling

Validation

Commands and Results

  • python -m pytest families/stable_diffusion/tests/test_reference.py apps/aiperf_qual/tests/test_request_contract.py -q: before 3 failed/4 passed, after 7 passed, independently rerun on the committed head.
  • python -m pytest apps/aiperf_qual/tests -q: 536 passed, 1 inherited inventory failure. Both original failures reproduce on unchanged base; request preflight now passes.
  • Real catalog→suite→payload resolution preserves pinned SDXL and SD1.5 controls. Actual Diffusers config/scheduler API and checkpoint geometry checked on CPU without weights.
  • python -m pytest tools/tests/test_architecture.py tools/tests/test_family_impact.py tools/tests/test_pr_metadata.py -q: 94 passed. Catalog, impact, legal headers, Ruff and whitespace checks passed.

Hardware, Environment, and Revisions

Head 7c57f77c8ea5dbabd54ce81ac17dece332272273, base a579f718410576264d382d13cca1e55391f37e79. Windows/Python 3.13 and Ubuntu WSL/Python 3.12 CPU checks. Metadata API probe: Diffusers 0.42.1, Torch 2.12.0+cpu. Existing checkpoint stabilityai/sdxl-turbo@71153311d3dbb46851df1931d3ca6e939de83304 is unchanged.

Not Run / Remaining Gaps

Full family collection cannot import TensorRT locally; two opt-in E2Es were skipped. No GPU/native build, checkpoint inference, actual compilation, image parity, multi-step ancestral replay, or performance qualification. Pipeline execution tests use controlled Diffusers/Torch doubles.

Contributor Self-Review

  • I have completed a self-review of this change.

Notes For Future Readers

Please review the unchanged image-inventory test's missing SDXL expectation separately; it remains failing rather than relaxing its assertion here. The existing checkpoint has Stability AI Community License terms, not Apache-2.0 weights; project owners must confirm applicable use/redistribution obligations. This change redistributes no weights/assets and changes no license or package requirement. Review the family YAML, adapter selection, then regressions.

Risk level

  • Low
  • Medium
  • High

Reference loading changes within one family; target-hardware numerical qualification remains unrun.

Use the checkpoint pipeline and family sampler for SDXL reference requests. State one-step zero-guidance controls and family-owned latent replay for the existing SDXL-Turbo profile.

Signed-off-by: JiaxinD <djx2048@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a579f71 and 7c57f77.

Files selected for processing: 3
  • families/stable_diffusion/reference/adapter.py
  • families/stable_diffusion/tests/aiperf/sdxl-turbo.yaml
  • families/stable_diffusion/tests/test_reference.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Summary

Summary

The SDXL-Turbo profile now sets one inference step and zero guidance and CFG scales. It uses the Stable Diffusion family’s reference and latent replay adapter.

The adapter selects StableDiffusionXLPipeline with EulerAncestralDiscreteScheduler for SDXL checkpoints. Other checkpoints retain StableDiffusionPipeline with DDIMScheduler. The adapter also replays caller-provided latents and reports the selected sampler.

New mocked tests cover SD and SDXL in eager and compile modes. They check scheduler selection, revision forwarding, guidance settings, latent reshaping, and output metadata.

Architecture impact

  • Family-owned files: Changes are limited to the Stable Diffusion reference adapter, SDXL-Turbo qualification configuration, and family tests.
  • Shared surfaces: No shared application or core implementation changes are shown in the supplied evidence.
  • Dependency direction: The qualification service loads the family adapter. No new dependency from family code to application or core code is shown.
  • Affected consumers: The adapter serves the SDXL-Turbo reference and latent replay paths. The existing SD1.5 profile also points to this adapter, and the new tests check that its DDIM behavior remains.
  • Unresolved blast radius: Tests use mocked pipeline and scheduler implementations. They do not establish compatibility with actual Diffusers checkpoints or runtime inference.
  • Review outcome: HUMAN REVIEW REQUIRED. The supplied evidence does not resolve actual runtime compatibility. No current review findings or severity counts were supplied, so counts are unavailable.

Validation

The PR reports 7 passing targeted tests, 536 passing broader qualification tests with one inherited inventory failure, and 94 passing architecture, family-impact, and metadata tests. Full family collection could not import TensorRT locally. Two opt-in E2E tests were skipped. GPU/native build, checkpoint inference, actual compilation, image parity, multi-step ancestral replay, and performance qualification were not run.

The reference adapter now selects the Stable Diffusion XL pipeline and Euler Ancestral scheduler for XL configurations. Other configurations retain the Stable Diffusion pipeline and DDIM scheduler. The change adds an SDXL Turbo AIPerf configuration and tests for both pipeline types in eager and compile modes.

Changes: Stable Diffusion adapter support

Layer / File(s) Summary
Pipeline and sampler selection
families/stable_diffusion/reference/adapter.py, families/stable_diffusion/tests/aiperf/sdxl-turbo.yaml, families/stable_diffusion/tests/test_reference.py
The adapter selects pipeline and scheduler options based on the configured pipeline name and reports the selected sampler. The SDXL Turbo configuration uses the reference adapter. Parametrized tests check pipeline and scheduler selection, invocation inputs, and returned metadata in eager and compile modes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: yifeif-nv, chaofengw-nv

Merge Risk: Minimal · up to 7c57f

No concrete merge-blocking issue was established. Actual checkpoint inference and image parity remain unverified.

Pre-merge checks: Passed 8 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 8 passed
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: wiring SDXL qualification for Stable Diffusion.
Description check Passed The description covers the required background, exit criteria, implementation, change categories, validation results, environment, remaining gaps, self-review, notes, and risk level. It clearly identi…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Family Ownership Boundary Passed No cross-family dependency is introduced. The authoritative diff changes only three files under families/stable_diffusion. The new profile points both adapters to `families/stable_diffusion/referenc…
Shared Semantic Neutrality Passed PASS. The pull request changes only families/stable_diffusion/reference/adapter.py, families/stable_diffusion/tests/aiperf/sdxl-turbo.yaml, and families/stable_diffusion/tests/test_reference.py.…
Benchmark Validation Integrity Passed PASS. The changed reference path preserves the measurement boundary: host.timed(lambda: self.pipe(**kwargs).images) remains the only model-call timing region, and latent loading, `tensor_observation…
Shared Change Blast Radius Passed The pull request changes only three family-owned paths under families/stable_diffusion: the reference adapter, the SDXL-Turbo profile, and the family reference test. It does not change shared code, …
Full details: Description check

Explanation

The description covers the required background, exit criteria, implementation, change categories, validation results, environment, remaining gaps, self-review, notes, and risk level. It clearly identifies unrun GPU and inference qualification work.

Full details: Family Ownership Boundary

Explanation

No cross-family dependency is introduced. The authoritative diff changes only three files under families/stable_diffusion. The new profile points both adapters to families/stable_diffusion/reference/adapter.py (yaml lines 6-8), and the manifest assigns sdxl-turbo to stable_diffusion. The adapter's new diffusers imports and SD/SDXL dispatch (lines 27-40) use an external shared library, not code owned by another repository family. The test imports only families.stable_diffusion.reference.adapter. No central registry or file outside this family changes.

Full details: Shared Semantic Neutrality

Explanation

PASS. The pull request changes only families/stable_diffusion/reference/adapter.py, families/stable_diffusion/tests/aiperf/sdxl-turbo.yaml, and families/stable_diffusion/tests/test_reference.py. These are model-owned family reference and test/config paths. The SDXL pipeline, scheduler, sampler, and replay controls are selected inside the stable-diffusion family adapter. No shared code outside the excluded family-owned directories changed.

Full details: Benchmark Validation Integrity

Explanation

PASS. The changed reference path preserves the measurement boundary: host.timed(lambda: self.pipe(**kwargs).images) remains the only model-call timing region, and latent loading, tensor_observation, artifact writing, synchronization, and invocation serialization remain outside that boundary as defined by the shared reference backend. SD and SDXL use the same control flow, output observation, timing value, and invocation metadata path. The new parameterized regression covers both pipelines in eager and compile modes, including controls, caller latents, output shape, timing, and sampler metadata. No changed code alters task/sample accounting or aggregation.

Full details: Shared Change Blast Radius

Explanation

The pull request changes only three family-owned paths under families/stable_diffusion: the reference adapter, the SDXL-Turbo profile, and the family reference test. It does not change shared code, contracts, tooling, catalogs, examples, benchmarks, or shared validation infrastructure. The custom failure condition does not apply to this family-local change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Comment @coderabbitai help to get the list of available commands.

@JiaxinD

JiaxinD commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

@yifeif-nv Community CPU / Required, Stable and Dev passed for 7c57f77. Please trigger protected CI for this head.

@JiaxinD
JiaxinD marked this pull request as ready for review October 31, 2026 00:58
@JiaxinD
JiaxinD requested a review from yifeif-nv as a code owner October 31, 2026 00:58

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant