Skip to content

Fix UnboundLocalError in get_parameter_dtype from a shadowed tuple builtin - #14579

Open
Om-singhaI wants to merge 1 commit into
huggingface:mainfrom
Om-singhaI:fix/get-parameter-dtype-tuple-shadowing
Open

Fix UnboundLocalError in get_parameter_dtype from a shadowed tuple builtin#14579
Om-singhaI wants to merge 1 commit into
huggingface:mainfrom
Om-singhaI:fix/get-parameter-dtype-tuple-shadowing

Conversation

@Om-singhaI

@Om-singhaI Om-singhaI commented Aug 24, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes #13789

get_parameter_dtype in src/diffusers/models/modeling_utils.py raises UnboundLocalError on every
call that reaches its nn.DataParallel fallback. ModelMixin.dtype is a thin wrapper around that
function, so a model that reads self.dtype inside forward dies under torch.nn.DataParallel.
UNet2DModel does exactly that at src/diffusers/models/unets/unet_2d.py:292, which is the path in
the issue report:

File ".../diffusers/models/unets/unet_2d.py", line ..., in forward
    t_emb = t_emb.to(dtype=self.dtype)
File ".../diffusers/models/modeling_utils.py", line ..., in dtype
    return get_parameter_dtype(self)
File ".../diffusers/models/modeling_utils.py", line ..., in get_parameter_dtype
    def find_tensor_attributes(module: nn.Module) -> list[tuple[str, Tensor]]:
UnboundLocalError: cannot access local variable 'tuple' where it is not associated with a value

The failure has nothing to do with multiple GPUs. It only needs a module whose tensors sit in
__dict__ instead of in _parameters or _buffers, which is precisely what
torch.nn.parallel.replicate produces: it empties _parameters on each replica, then re assigns the
broadcast copies with setattr. Because those copies are plain tensors rather than nn.Parameter,
they land in replica.__dict__, named_parameters() and buffers() both come back empty, and
get_parameter_dtype falls through to its last branch.

Reproduction

Single process, CPU only, no accelerator needed. On main at 58eb52c08:

import sys

import torch
from torch import nn

from diffusers.models.modeling_utils import get_parameter_dtype

print("python:", sys.version.split()[0], "| torch:", torch.__version__)

model = nn.Linear(4, 4).to(torch.float16)
print("original dtype          :", get_parameter_dtype(model))

# what torch.nn.parallel.replicate() builds for one device: _parameters is emptied,
# then the broadcast copies are re assigned with setattr, so they are plain tensors
# sitting in replica.__dict__ rather than in replica._parameters.
replica = model._replicate_for_data_parallel()
for key, param in model._parameters.items():
    setattr(replica, key, param.detach())

print("replica named_parameters:", list(replica.named_parameters()))
print("replica buffers         :", list(replica.buffers()))
print("replica __dict__ tensors:", [k for k, v in replica.__dict__.items() if torch.is_tensor(v)])
print("replica dtype           :", get_parameter_dtype(replica))
python: 3.10.6 | torch: 2.9.1
original dtype          : torch.float16
replica named_parameters: []
replica buffers         : []
replica __dict__ tensors: ['weight', 'bias']
Traceback (most recent call last):
  File "repro.py", line 23, in <module>
    print("replica dtype           :", get_parameter_dtype(replica))
  File "src/diffusers/models/modeling_utils.py", line 205, in get_parameter_dtype
    def find_tensor_attributes(module: nn.Module) -> list[tuple[str, Tensor]]:
UnboundLocalError: local variable 'tuple' referenced before assignment

Note where the traceback points: line 205, the def statement, not the loop below it. With this PR
the same script prints replica dtype : torch.float16.

Root cause

    def find_tensor_attributes(module: nn.Module) -> list[tuple[str, Tensor]]:
        tuples = [(k, v) for k, v in module.__dict__.items() if torch.is_tensor(v)]
        return tuples

    gen = parameter._named_members(get_members_fn=find_tensor_attributes)
    last_tuple = None
    for tuple in gen:
        last_tuple = tuple

for tuple in gen: binds tuple, so the compiler treats tuple as a function local for the entire
scope of get_parameter_dtype, including the lines above the loop. modeling_utils.py has no
from __future__ import annotations, so the return annotation on the nested def is a normal
expression evaluated when the def statement executes. Evaluating list[tuple[str, Tensor]] reads
tuple, which at that moment is an unbound fast local, and Python raises before the loop can ever
bind it.

The compiled code object shows it directly:

co_varnames: ('parameter', 'submodule', 'registry', 'hook', 'last_dtype', 'param', 'buffer',
              'tuple', 'find_tensor_attributes', 'gen', 'last_tuple')
co_names   : (..., 'list', 'str', 'Tensor', '_named_members')
'tuple' is a fast local: True

list, str and Tensor are global lookups in co_names, as expected. tuple is not: it sits in
co_varnames with the locals.

get_parameter_device has the same nested def with the same annotation but binds first_tuple
rather than tuple, so it never shadows the builtin and is not affected. Its co_varnames are
('parameter', '_get_group_onload_device', 'parameters_and_buffers', 'find_tensor_attributes', 'gen', 'first_tuple').

When it regressed

git blame on the two lines splits them across two commits:

7c2f0afb1c ... (YiYi Xu     2024-12-22 204)     # For nn.DataParallel compatibility in PyTorch > 1.5
2843b3d37a ... (Sayak Paul  2026-02-13 205)     def find_tensor_attributes(module: nn.Module) -> list[tuple[str, Tensor]]:
7c2f0afb1c ... (YiYi Xu     2024-12-22 211)     for tuple in gen:

The for tuple in gen: loop arrived in 7c2f0afb1c ("update get_parameter_dtype", #10342) and was
harmless for over a year, because the annotation then read List[Tuple[str, Tensor]] and Tuple was
a module global that nothing shadowed.

2843b3d37a ("Sunset Python 3.8 & get rid of explicit typing exports where possible", #12524)
lowercased it:

-        def find_tensor_attributes(module: torch.nn.Module) -> List[Tuple[str, Tensor]]:
+        def find_tensor_attributes(module: torch.nn.Module) -> list[tuple[str, Tensor]]:
@@
-    def find_tensor_attributes(module: nn.Module) -> List[Tuple[str, Tensor]]:
+    def find_tensor_attributes(module: nn.Module) -> list[tuple[str, Tensor]]:

That is the moment the annotation started colliding with the loop variable. get_parameter_device
got the identical edit in the same commit and stayed correct only because it never binds the name the
annotation reads: it takes first_tuple = next(gen) rather than looping over tuple.

Affected Python versions

Measured on the standalone pattern, so the interpreter is the only variable:

  • CPython 3.10.6 raises UnboundLocalError. tuple is in co_varnames, co_cellvars is empty.
  • CPython 3.13.15 raises UnboundLocalError. Same code object shape.
  • CPython 3.14.3 returns normally. tuple is in co_cellvars and the function has __annotate__.

3.14 hides the bug. Under PEP 649 the annotation is compiled into a separate __annotate__ code
object that is not evaluated when the def statement runs, tuple becomes a cell variable rather
than a fast local, and the call completes. Anyone checking this on 3.14 will not reproduce it.

That matters because the affected range is the supported range:

  • setup.py declares python_requires=">=3.10.0".
  • .github/workflows/pr_tests.yml pins python-version: "3.10" for the PR test jobs.

So every supported interpreter from 3.10 through 3.13 hits this, CI itself runs on one of them, and
the reporter in #13789 is on 3.11.13.

The fix

Rename the loop variable. Nothing else changes, and the annotation resolves tuple as the builtin
again.

     gen = parameter._named_members(get_members_fn=find_tensor_attributes)
     last_tuple = None
-    for tuple in gen:
-        last_tuple = tuple
-        if tuple[1].is_floating_point():
-            return tuple[1].dtype
+    for t in gen:
+        last_tuple = t
+        if t[1].is_floating_point():
+            return t[1].dtype

last_tuple is left alone because it shadows nothing, and get_parameter_device is left alone
because it was never broken.

After the patch, co_varnames no longer contains tuple, and tuple joins list, str and
Tensor in co_names as a global lookup.

Test

Two tests in TestModelUtils in tests/models/test_modeling_common.py, both CPU only and offline.
The first puts a module into the same state torch.nn.parallel.replicate leaves a replica in and
asserts the dtype comes back, parametrized over float16, bfloat16 and float32; the second
covers the other exit from the same branch, where no tensor is floating point and the last one found
is returned.

That replica state is rebuilt by hand rather than obtained from torch.nn.parallel.replicate
itself, because replicate broadcasts through CUDA and the CPU test job has no device to broadcast
to. See non blocking item 3 in the self review notes below, and say the word if you would rather
have a real torch.nn.DataParallel test gated on @require_torch_multi_gpu alongside this one.

With the source change reverted and the tests kept:

src/diffusers/models/modeling_utils.py:205: UnboundLocalError
FAILED tests/models/test_modeling_common.py::TestModelUtils::test_get_parameter_dtype_on_data_parallel_replica[dtype0]
FAILED tests/models/test_modeling_common.py::TestModelUtils::test_get_parameter_dtype_on_data_parallel_replica[dtype1]
FAILED tests/models/test_modeling_common.py::TestModelUtils::test_get_parameter_dtype_on_data_parallel_replica[dtype2]
FAILED tests/models/test_modeling_common.py::TestModelUtils::test_get_parameter_dtype_falls_back_to_non_floating_point_tensor_attribute
4 failed, 17 deselected in 2.64s

With the source change in place, the whole module is green:

$ python -m pytest tests/models/test_modeling_common.py
17 passed, 4 skipped in 12.23s

ruff check and ruff format --check (0.9.10, the version pinned in setup.py) pass on both
changed files and across examples scripts src tests utils setup.py.

Self review notes

Checked the diff against .ai/references/review-rules.md, plus code_style.md, models.md, testing.md and pitfalls.md. Small thing for maintainers: SKILL.md points at references/... relative to itself, but the skill directory has no references/ subdirectory, so those files actually live at .ai/references/.

Blocking issues

None.

Non blocking issues

  1. Typo in a new test comment.
    The comment reads "re assigns" where the word is reassigns.
    tests/models/test_modeling_common.py:284
    Impact: cosmetic only. The comment is otherwise accurate and stands alone for a future reader, which is what references/review-rules.md asks of comments under "Ephemeral context".
    No rule citation; this is a plain typo.

  2. Single letter loop variable.
    t is the smallest edit that removes the shadow and it is correct. The repo does state a preference for spelled out names, and the sibling function binds first_tuple at modeling_utils.py:161, so current_tuple or name_and_tensor would read more consistently with the file. Every one of these choices fixes the bug equally.
    src/diffusers/models/modeling_utils.py:211
    Impact: readability only, no behavior difference.
    Per references/models.md: "Prefer descriptive variable names over short ones. For example, prefer spelling out query over q."

  3. The replica in the test is hand built rather than produced by torch.nn.parallel.replicate.
    The helper calls the real private Module._replicate_for_data_parallel() and then performs the setattr half of torch.nn.parallel.replicate itself, reading module._parameters directly. Both are private torch APIs. If torch ever changes how replicate assigns broadcast copies, this test stays green while real nn.DataParallel breaks. The reason for hand building it is legitimate: torch.nn.parallel.replicate broadcasts through CUDA, so it cannot run in the CPU test job.
    tests/models/test_modeling_common.py:283
    Impact: the test pins the shape of the bug rather than the real nn.DataParallel path.
    Per references/testing.md: "don't monkeypatch a component method (e.g. the scheduler's set_timesteps) just to capture what the code under test passed to it — that only verifies the caller against itself, not against the real method's contract. Call the real component and assert on its resulting state."

  4. Documentation impact: none.
    get_parameter_dtype is not exported from src/diffusers/__init__.py or src/diffusers/models/__init__.py, no public signature, argument or default changed, and behavior for every module that has parameters or buffers is unchanged. No page under docs/ describes this fallback. Flagged explicitly because references/review-rules.md asks that docs be scanned rather than assumed.

  5. Suggestion for a maintainer: write this gotcha down.
    The ruff selectors in pyproject.toml are ["C", "E", "F", "I", "W"] with F402 in the ignore list, and flake8-builtins (A001) is not enabled, so no configured linter can catch a builtin shadowed by a loop variable. The failure only surfaces here because a nested annotation is evaluated at runtime in the same scope, which is easy to miss on review. A line in .ai/references/code_style.md would give the next contributor this for free. Raised here rather than changed in the diff, because the contributor guide says .ai/ is maintained by core maintainers and contributors should flag rather than edit.
    Per references/review-rules.md: "If the review turns up a rule, pattern, or common gotcha that isn't written down yet — especially one the author got wrong or that you had to reason out — propose adding it to the relevant agent guide".

Dead code (advisory)

Location Status Reason
src/diffusers/models/modeling_utils.py:211 (the renamed loop) Used Reached whenever a module has an empty _parameters, no buffers, and tensors in __dict__. ModelMixin.dtype (modeling_utils.py:1963) routes here, and shipped models reach it from inside forward on plain submodules: transformer_minimax_h3.py:129,152,628,629,630,642,658, transformer_skyreels_v2.py:361, transformer_motif_video.py:437, autoencoder_kl_minimax_h3.py:862,887, autoencoder_kl_minimax_h3_audio.py:611,644, autoencoder_cosmos3_audio.py:594. Under nn.DataParallel each of those submodules is a replica, so this is a live forward path, not just a .dtype property read.
src/diffusers/models/modeling_utils.py:216 to :218 (the last_tuple fallback) Used Reached when __dict__ holds only non floating point tensors. Covered by the new test at tests/models/test_modeling_common.py:302.
tests/models/test_modeling_common.py:283 _as_data_parallel_replica Used One caller, the test at tests/models/test_modeling_common.py:296.
tests/models/test_modeling_common.py:293 and :302 Used Collected by pytest from TestModelUtils, which is a plain class and not a unittest.TestCase, so the @pytest.mark.parametrize at line 292 applies as intended.
src/diffusers/models/modeling_utils.py:216, the path where last_tuple is None Not dead, pre existing The function falls off the end and returns None. Outside this diff; see "Leave for the actual review" below.

Summary

Verdict: READY.

The diff does one thing and does it correctly. The stated root cause holds up under isolation: the same shape raises UnboundLocalError with for tuple in gen: and returns the expected dtype after the rename, and the two new tests exercise both exits of the repaired branch. Nothing added is defensive, unused or out of scope, no # Copied from block is touched so make fix-copies has nothing to propagate, and all added lines sit inside the configured ruff line length.

Fix before submitting

  • Nothing in the diff. There are no blocking issues and no dead code to remove.
  • One verification step that is not a code change: run pytest tests/models/test_modeling_common.py -k get_parameter_dtype in a diffusers environment before opening the PR. Both branches were checked by reproducing them standalone against torch 2.13, but the repo suite itself was not executed during this review.
  • Optionally fold in the one word typo at tests/models/test_modeling_common.py:284, since it costs nothing.

Leave for the actual review

  • Non blocking 2, the loop variable name. t is correct and minimal, and a maintainer may simply prefer a spelled out name. Not worth guessing at.
  • Non blocking 3, whether a real nn.DataParallel test gated on @require_torch_multi_gpu should accompany the CPU one, given that the CPU test necessarily reconstructs the replica state by hand.
  • Non blocking 5, the .ai/references/code_style.md addition, which only a maintainer can make.
  • Pre existing and deliberately untouched: when a module has no parameters, no buffers and no tensor attributes at all, get_parameter_dtype falls off the end of the function and returns None (src/diffusers/models/modeling_utils.py:216), so ModelMixin.dtype would hand back None rather than the torch.dtype its annotation promises. That predates this branch, and fixing it would turn a four line bug fix into a behavior change. Happy to open a separate issue if you would like it addressed.

Before submitting

Who can review?

@sayakpaul @DN6

`get_parameter_dtype` loops with `for tuple in gen:`, which makes `tuple` a
function local for the whole scope. The nested `find_tensor_attributes` declares
`list[tuple[str, Tensor]]` as its return annotation, and since this module has no
`from __future__ import annotations`, that annotation is evaluated when the `def`
statement runs. `tuple` is then read as an unbound fast local, so the call raises
`UnboundLocalError` before the loop ever binds it.

The failing path is the fallback for modules whose tensors live in `__dict__`
rather than in `named_parameters()` or `buffers()`, which is exactly what
`torch.nn.parallel.replicate` builds for each device. `ModelMixin.dtype` routes
here, so any model that reads `self.dtype` in `forward` raises under
`torch.nn.DataParallel`.

Renaming the loop variable removes the shadow, so the annotation resolves `tuple`
as the builtin again. `get_parameter_device` is unaffected because it already
binds `first_tuple`.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UNet2DModel dtype property fails under nn.DataParallel with UnboundLocalError in get_parameter_dtype

1 participant