Skip to content

Fix StableDiffusion3Pipeline.skip_guidance_layers raising AttributeError - #14731

Open
yupengtang wants to merge 1 commit into
huggingface:mainfrom
yupengtang:fix-sd3-skip-guidance-layers-property
Open

Fix StableDiffusion3Pipeline.skip_guidance_layers raising AttributeError#14731
yupengtang wants to merge 1 commit into
huggingface:mainfrom
yupengtang:fix-sd3-skip-guidance-layers-property

Conversation

@yupengtang

Copy link
Copy Markdown

What does this PR do?

StableDiffusion3Pipeline.skip_guidance_layers raises AttributeError, before or after a call:

@property
def skip_guidance_layers(self):
    return self._skip_guidance_layers

__call__ stores the other guidance knobs on self so their properties can read them back, but skip_guidance_layers was left out of that block:

self._guidance_scale = guidance_scale
self._skip_layer_guidance_scale = skip_layer_guidance_scale
self._clip_skip = clip_skip
self._joint_attention_kwargs = joint_attention_kwargs
self._interrupt = False

grep -rn "_skip_guidance_layers = " src/ returns nothing, so the name is never assigned anywhere in the library. skip_guidance_layers stays a local in __call__ and does reach the transformer, so skip layer guidance itself works. Only the property is broken.

Adding the missing assignment is the whole change. StableDiffusion3Pipeline.__call__ is not a # Copied from source for any other pipeline, so nothing else moves.

Testing

tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3.py::TestStableDiffusion3Pipeline::test_skip_guidance_layers already runs the feature both ways, so it now also reads the property back. Two assertions, no new fixture.

  • That test on main: fails with AttributeError: 'StableDiffusion3Pipeline' object has no attribute 'skip_guidance_layers'. With this branch: passes. The message names the property rather than the missing underscore attribute, because the getter's AttributeError falls through to nn.Module.__getattr__, which probably helped this go unnoticed.
  • The whole file: 1 failed, 20 passed, 17 skipped on both main and this branch. The failure is test_inference, which compares against a hardcoded expected slice at atol=1e-3 and misses on my CPU box on main too.
  • ruff check and ruff format --check on both changed files with the 0.9.10 pinned in setup.py: clean.
  • make fix-copies is unaffected, since nothing copies from __call__.

Ran on Python 3.13, torch 2.11.0, Linux, CPU.

Self-review

Against .ai/references/review-rules.md:

  • Correctness: the added line mirrors the four assignments around it and uses the parameter already in scope. No behaviour outside the property changes, which is why the output comparisons in that test are untouched and still pass.
  • Ephemeral context: no comments added, nothing that refers to this PR or its review.
  • Copied code: checked that __call__ is not a # Copied from source before editing it.
  • Tests: extends the test that already covers this feature rather than adding a second pipeline build, per the dummy-component conventions in testing.md.
  • Documentation impact: skip_guidance_layers is documented as a __call__ argument in the docstring, which is accurate and unchanged. Nothing under docs/ mentions the property.
  • Suggestion, not blocking: _skip_layer_guidance_scale is the mirror image of this, assigned in __call__ but with no property, while skip_layer_guidance_start and skip_layer_guidance_stop have neither. If the intent is for every guidance knob to be readable back, those are the remaining gaps. I left them alone since only the missing assignment causes a failure today.

`__call__` stores the other guidance knobs on `self` so the matching
properties can read them back:

    self._guidance_scale = guidance_scale
    self._skip_layer_guidance_scale = skip_layer_guidance_scale
    self._clip_skip = clip_skip
    ...

`skip_guidance_layers` was left out, but the property that reads
`self._skip_guidance_layers` exists, so `pipe.skip_guidance_layers` raises
`AttributeError` whether or not a call has run. Nothing in `src/diffusers`
assigns that name.

The existing `test_skip_guidance_layers` now checks the property alongside
the outputs it already compares.
@github-actions github-actions Bot added size/S PR with diff < 50 LOC tests pipelines labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Hi @yupengtang, 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.

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

Labels

pipelines size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant