Skip to content

Add Group Offloading Summary Utility - #14702

Open
dg845 wants to merge 3 commits into
mainfrom
group-offloading-inspection-utilities
Open

Add Group Offloading Summary Utility#14702
dg845 wants to merge 3 commits into
mainfrom
group-offloading-inspection-utilities

Conversation

@dg845

@dg845 dg845 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

This PR adds a group offloading summary utility _get_group_offload_summary which outputs a string showing the offloading groups of a module. The motivation is to allow users to easily see the created offloading groups and to aid in debugging group offloading bugs. It can also be used for more informative test messages (an example is in GroupOffloadTesterMixin._run_group_offload_inference in this PR).

Before submitting

  • Did you use an AI agent (Claude Code, Codex, Cursor, etc.) to help with this PR? If so:
    • Did you read the Coding with AI agents guide?
    • Did you run the self-review skill on the diff?
    • Did you share the final self-review notes in the PR description or a comment?
  • Did you read the contributor guideline?
  • Did you read our philosophy doc? (important for complex PRs)
  • Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?
  • Are you the author (or part of the team) of the model/pipeline (only applicable for model/pipeline related PRs)?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@sayakpaul
@DN6

dg845 and others added 2 commits September 4, 2026 09:06
Group offloading leaves no way to ask what grouping it installed: the
structure lives on the `GroupOffloadingHook` in each participating
submodule's `_diffusers_hook` registry, several submodules share one
`ModuleGroup`, and the groups hold module objects rather than names. So a
device-mismatch failure names only the layer that tripped over it, never the
group whose weights were never onloaded.

Add `_get_group_offload_summary` next to the existing introspection
cluster (`_get_top_level_group_offload_hook` and friends). It dedupes the
groups and resolves the module objects back to qualified names, reporting
each group against the module whose `forward` actually brings its weights
over. That is not always the group's own leader: with `use_stream=True`,
lazy prefetch sets `onload_self = False` and the group is onloaded by
whichever group names it as `next_group`, so the prefetch chain is read off
the per-module hooks rather than the shared group. That chain is wired only
once a module's `forward` has completed once, which the docstring calls out
— a module that had not run yet when the failure hit reports every group as
onloading itself, and reading that as "this module is not prefetching" would
be wrong.

A group can reach modules that are not under the one being walked, so calling
this on a submodule of an offloaded model — one component of a pipeline, say
— finds the group its siblings belong to and cannot name them. Those are
reported as `<outside this module>` rather than raising, since a debugging
aid that dies when pointed at the thing being debugged is no use.

It stays private. What a component reports would shift the day it gains
`_group_offload_block_modules`, and reporting the grouping is all this can
soundly do — whether a component is safe to offload at block level depends
on which entry points the pipeline drives it through, which the module graph
alone does not say. A component can group normally and still break:
`PriorTransformer` splits its `transformer_blocks` into per-block groups yet
reads `clip_mean` / `clip_std` from the root group in `post_process_latents`,
and `AutoencoderKL` groups entirely into its `down_blocks` / `up_blocks`
while `decode` bypasses `forward`.

Use it in the pipeline group offload tests, where a `RuntimeError` from the
offloaded run is now re-raised with the grouping of every offloaded component
attached.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_get_group_offload_summary` is only read once something has already gone
wrong, so a regression in it is invisible: a summary is still produced, it is
just wrong, and it sends the reader to the wrong module. Nothing else in the
suite would notice.

Cover what it claims rather than how it formats it — the number of groups,
and which module's `forward` brings each one over — asserting against the
groups read straight off the hooks. Only "no group offloading applied" and
"prefetched by" are matched as text, so the layout stays free to change.

The two cases that carry their weight are prefetching, which is reported only
once the first forward has wired the chain, and summarizing a submodule whose
group reaches outside it. Both were live bugs in the helper: reverting either
fix fails exactly its own test and no other.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added tests hooks size/M PR with diff < 200 LOC labels Sep 4, 2026
@dg845

dg845 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author
Self-Review Report

Self-review

Ran the /self-review rubric (.ai/references/review-rules.md, plus code_style.md and testing.md) over the full branch diff. Verdict: ready. No blocking issues remain; the three items below are judgement calls I'd rather put to a reviewer than settle alone.

What this adds

commit what
Add a group offloading introspection helper _get_group_offload_summary in src/diffusers/hooks/group_offloading.py, plus its use in the pipeline group-offload tests
Test the group offloading summary four tests in tests/hooks/test_group_offloading.py

Group offloading installs its grouping on the GroupOffloadingHook in each submodule's _diffusers_hook registry, several submodules share one ModuleGroup, and groups hold module objects rather than names. So when a group's weights are never onloaded, the resulting error names only the layer that tripped over it. The helper renders the grouping; _run_group_offload_inference attaches it to any RuntimeError from the offloaded run.

Each group is reported against the module whose forward actually brings its weights over. That is not always its own leader — with use_stream=True, lazy prefetch sets onload_self = False and the group is onloaded by whichever group names it as next_group:

use_stream=False                          use_stream=True, after the first forward
  onloaded by '<root>'   [head]             onloaded by '<root>'      [head]
  onloaded by 'blocks.0' [blocks.0]         prefetched by '<root>'    [blocks.0]
  onloaded by 'blocks.1' [blocks.1]         prefetched by 'blocks.0'  [blocks.1]
  onloaded by 'blocks.2' [blocks.2]         prefetched by 'blocks.1'  [blocks.2]

Non-blocking findings

1. The test oracle shares the implementation's algorithm. installed_groups (tests/hooks/test_group_offloading.py:721) reimplements the same registry walk and id(hook.group) dedupe as the code under test, so a shared misconception about which hooks exist would be invisible to test_reports_one_line_per_installed_group. It is genuinely independent where it matters most: test_reports_prefetching_only_once_the_chain_is_wired reads group.onload_self straight off the hook, while the implementation derives prefetching through a separate map. Inherent to the domain rather than fixable.

2. TestGroupOffloadSummary has no teardown_method. It is the only accelerator-touching class in the file without the gc.collect() / backend_empty_cache / backend_reset_peak_memory_stats teardown that TestGroupOffload has. I checked whether this can contaminate the file's memory assertions and it cannot — test_offloading_forward_pass's inner run_forward resets peak stats before each measurement and compares relatively, so a constant residual preserves the inequalities. Consistency, not a flake.

3. The handler decorates every RuntimeError, not only device mismatches. tests/pipelines/testing_utils/memory.py:377. A shape error or a cuDNN gap also gets an offload-grouping block appended. Harmless — the grouping is factual regardless, and the original message is preserved as a prefix so skip_if_no_cudnn_engine's substring match still works — but noisier than the comment implies.

Fixed during review

  • The summary misreported prefetched groups. It printed onloaded by <own leader> unconditionally. Measured on a 3-block model under use_stream=True, three of four lines were wrong, on exactly the path where a device mismatch is hardest to reason about.
  • KeyError when summarizing a submodule. name_of is built from the walked module, but a group with num_blocks_per_group > 1 holds that module's siblings, so the lookup missed. Calling the helper on one component of an offloaded model — the natural thing to do while debugging — crashed. Members outside the walked module are now reported as <outside this module>.
  • Five of nine fields on the original GroupOffloadGroupInfo dataclass were never read. Inlining the walk into the single consumer removed the dataclass and the dead fields, and turned onload_self from unused into load-bearing.
  • A second return value the only caller discarded, in _enable_group_offload_on_components.

What the tests actually pin

The helper only runs once something has already failed, so a regression in it is invisible — a summary is still produced, it is just wrong, and it sends the reader to the wrong module. Nothing else in the suite would notice. Each fix was reverted in turn to confirm the tests catch it:

reverted fix test that fails
direct name_of[...] lookups (the KeyError) test_summarizes_a_submodule_whose_group_reaches_outside_it
unconditional "onloaded by" test_reports_prefetching_only_once_the_chain_is_wired
silently dropping unnameable members test_summarizes_a_submodule_whose_group_reaches_outside_it

Each fails exactly one test and leaves the others green. The tests assert against groups read straight off the hooks; only "no group offloading applied" and "prefetched by" are matched as text, so the layout stays free to change.

Open questions

Should the hooks / groups split collapse? The walk keeps every hook to build the prefetch chain and dedupes only for display. That distinction is provably unobservable today: it needs a group with more than one hook and a prefetch chain, and group_offloading.py:723 coerces num_blocks_per_group = 1 whenever a stream is used, so the two never coexist. Collapsing the lists fails no test. I kept the general form because it is written against the data structure — next_group is a hook attribute, group is shared — rather than against a coercion 600 lines away that exists for unrelated reasons, and because if that restriction is ever lifted the deduped version silently reports prefetched groups as self-onloading. code_style.md's "no unused code paths" reads the other way, so I would take direction here.

Should the leading underscore come off? The original argument for keeping it private was that GroupOffloadGroupInfo mirrored ModuleGroup's fields and would freeze an implementation detail. That dataclass is gone; the function returns a plain str, whose content can change freely. If reaching for this while debugging a component is an intended use, the underscore is a sign saying "don't" on the thing people would want. Making it public would mean a hooks/__init__.py export and an entry in docs/source/en/api/utilities.md alongside apply_group_offloading.

Should there be a test for the exclusion lists? An earlier draft added one asserting that a config's block-level exclusions are entirely root-gated. I dropped it: the predicate is unsound. PriorTransformer splits its transformer_blocks into per-block groups yet reads clip_mean / clip_std from the root group in post_process_latents (18 of 50 parameters root-gated), and AutoencoderKL groups entirely into its down_blocks / up_blocks while decode bypasses forward (0 of 92). Both have exactly the bug the exclusions exist for, and both would have failed that check — it would have told you to remove a load-bearing exclusion. Whether a component is safe to offload at block level depends on which entry points the pipeline drives it through, which the module graph alone does not say.

Verification

  • tests/hooks/test_group_offloading.py — 42 passed
  • tests/pipelines/{ideogram4,pndm,ltx2} with -m "memory or group_offload" — 82 passed, 2 skipped, 11 xfailed, 1 xpassed (the xpass is the pre-existing documented pndm one)
  • ruff format and ruff check clean

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sayakpaul sayakpaul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in favor of such a utility but the testing seems dodgy to me.

Comment on lines +1028 to +1040
"""Render the offload grouping of a module as a short block, for use in assertion messages.

The grouping lives on the `GroupOffloadingHook` in each participating submodule's `_diffusers_hook` registry, and
several submodules share one `ModuleGroup`, so this dedupes the groups and resolves the module objects back to
qualified names. Each group is reported against the module whose `forward` actually brings its weights over: a
group with `onload_self=False` is onloaded by whichever group names it as `next_group`, not by its own leader.

The grouping is reported as of the moment this is called. With `use_stream=True` the prefetch chain is wired only
once a module's `forward` has completed once, so a module that has not run yet reports every group as onloading
itself, exactly as it would without a stream.

A group can span modules that are not under the one being walked — calling this on a submodule of an offloaded
model reaches the group its siblings belong to — so those are reported as `<outside this module>`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify 😅

)
summary = _get_group_offload_summary(model)

assert len(self.group_lines(summary)) == len(self.installed_groups(model))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a sensible test to check against the lines of summary and the number of installed groups. It is fragile. Better to assert against specific keywords per expectation.

torch.device(torch_device).type not in ["cuda", "xpu"],
reason="Test requires a CUDA or XPU device.",
)
def test_reports_prefetching_only_once_the_chain_is_wired(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's robust to check if prefetching happened based on a summary. It's better to install hooks and derive a mechanism based on that to perform these checks.

@sayakpaul
sayakpaul requested a review from DN6 September 4, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hooks size/M PR with diff < 200 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants