Skip to content

[TRTLLM-11412][feat] Add offloading support for visual_gen - #14095

Open
rahul-steiger-nv wants to merge 22 commits into
NVIDIA:mainfrom
rahul-steiger-nv:visual_gen_offloading
Open

[TRTLLM-11412][feat] Add offloading support for visual_gen#14095
rahul-steiger-nv wants to merge 22 commits into
NVIDIA:mainfrom
rahul-steiger-nv:visual_gen_offloading

Conversation

@rahul-steiger-nv

@rahul-steiger-nv rahul-steiger-nv commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds initial offloading support for tensorrt_llm/_torch/visual_gen.

The goal is to reduce peak GPU memory usage for memory-heavy visual generation pipelines by allowing selected model components, auxiliary modules, or guardrails to be offloaded when they are not actively needed during generation.

This is intended to help support larger visual generation workloads, longer video generation, multi-view generation, and lower-VRAM GPUs.

This PR is an initial implementation for the feature request in #13893.

Multi-GPU support and quantization compatibility are still being validated. There may also be some design changes after further discussion to make the implementation more general and easier to maintain upstream.

Test Coverage

Current testing:

  • Tested visual generation with offloading enabled on a single GPU.
  • Verified that peak GPU memory usage is reduced compared to running without offloading.
  • Verified that generation completes successfully with offloading enabled.

Planned / in-progress testing:

  • Multi-GPU execution.
  • Quantized model paths.
  • Additional validation of latency overhead.
  • Additional tests for configuration/error handling once the final design is agreed upon.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added CPU offloading support for Wan text-to-video inference with configurable stages targeting text encoder, VAE, and transformer components.
    • Added CUDA peak memory logging for per-request inference monitoring.
    • New CLI flags: --enable_offloading, --offload_stages, and --enable_cuda_memory_logging.
  • Documentation & Examples

    • Updated visual-generation documentation with CPU offloading capabilities and usage guidance.
    • Added example configurations demonstrating Wan T2V inference with CPU offloading for memory optimization.

Review Change Stack

Performance results

Measured with Wan-AI/Wan2.2-T2V-A14B-Diffusers at 480x832 resolution and 33 frames.

  • Without offloading: 73.96s total pipeline time, 68.54 GiB peak CUDA memory
  • With CPU offloading: 74.28s total pipeline time, 31.62 GiB peak CUDA memory
  • Offloading setup time: 15.29s

In this single-run measurement, CPU offloading reduced peak CUDA memory by 36.92 GiB, from 68.54 GiB to 31.62 GiB, which is about a 54% reduction. End-to-end pipeline time remained effectively flat, with only 0.32s additional runtime, or about 0.4% overhead, excluding the one-time offloading setup cost.

This enables Wan-AI/Wan2.2-T2V-A14B-Diffusers to run comfortably on L40 and A6000-class 48 GB GPUs. RTX 5090 support may be feasible with careful tuning, but 32 GB remains a marginal target due to allocator fragmentation and limited headroom at higher resolutions or frame counts.

@rahul-steiger-nv
rahul-steiger-nv marked this pull request as ready for review May 13, 2026 15:19
@rahul-steiger-nv
rahul-steiger-nv requested review from a team as code owners May 13, 2026 15:19
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR implements CPU offloading for visual-generation pipelines, enabling selective movement of text encoders, VAEs, and transformer blocks between GPU and pinned CPU storage to reduce peak GPU memory consumption. The change spans infrastructure (packing/staging manager), pipeline integration, model-specific wiring for WAN T2V, metadata materialization, memory logging, CLI/config updates, and comprehensive tests.

Changes

CPU Offloading for Visual Generation

Layer / File(s) Summary
ModuleOffloadManager and OffloadPipeline core
tensorrt_llm/_torch/visual_gen/offloading.py
ModuleOffloadManager packs aligned parameter/buffer bytes into per-group CPU storage with shared-tensor aliasing preservation; creates typed CPU/GPU views and manages a reusable GPU arena. OffloadPipeline normalizes stage config, maps stages to module groups, and provides context-based staging and cleanup.
Pipeline configuration and OffloadConfig
tensorrt_llm/visual_gen/args.py, tensorrt_llm/_torch/visual_gen/config.py, tensorrt_llm/visual_gen/__init__.py
Introduces OffloadConfig pydantic model with enable/device/pin_memory/stages fields. Extends DiffusionModelConfig and VisualGenArgs with offload_config and device fields. Re-exports OffloadConfig from visual_gen module.
BasePipeline offload methods and lifecycle
tensorrt_llm/_torch/visual_gen/pipeline.py
Adds offload state members, helper methods for stage resolution/normalization/discovery/filtering, offload_context() interface, and initialize_offload_pipeline() to construct/init OffloadPipeline post-weights. Disallows CUDA-graph when offloading configured. Updates transformer_components to return list[str].
WAN model offload stage defaults and setup
tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan.py, tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan_i2v.py
Defines single/two-transformer WAN offload stage groupings. Implements WanPipeline.default_offload_stages() to return CPU-only stages based on topology. Registers with enable_cuda_memory_logging default. Tightens transformer_components return type to list[str].
WAN model runtime offload context wrapping
tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan.py
Conditionally moves text_encoder and VAE to CPU during component loading. Wraps transformer calls with offload_context() using timestep-based stage selection. Wraps text_encoder and VAE calls in offload_context() for prompt encoding and conditioning/decode.
Load-time CPU materialization for offload modules
tensorrt_llm/_torch/visual_gen/pipeline_loader.py
Derives load-time CPU-offload modules from pipeline stages. Performs two-phase materialization: materializes selected modules on CPU, then remaining meta tensors on target device while excluding CPU-offload-owned tensors. Calls initialize_offload_pipeline() post-weights.
Per-request CUDA peak memory logging
tensorrt_llm/_torch/visual_gen/executor.py
DiffusionExecutor.process_request() resets CUDA peak stats before inference when enabled, logs torch.cuda.max_memory_allocated() after success or exception. Provides safe CUDA helpers with CPU fallbacks and warning on failure.
FP8 linear module compute device optimization
tensorrt_llm/_torch/modules/linear.py
FP8QDQLinearMethod.rescale_fused_weights() selects compute_device (CUDA when weights on CPU) for scale computations and per-shard rescaling, then copies results back to original weight device.
CLI flags and per-model example scripts
examples/visual_gen/visual_gen_wan_t2v.py, examples/visual_gen/models/wan_t2v.py
Adds CLI flags --enable_offloading, --offload_stages (comma-separated), and --enable_cuda_memory_logging. Constructs offload_config dict and wires into VisualGenArgs.
Example configs and documentation
examples/visual_gen/configs/*, examples/visual_gen/README.md, docs/source/models/visual-generation.md
Adds wan2.2-t2v-cpu-offload.yaml example config. Updates config header comments. Documents offloading in README (section, examples, common arguments table) and VisualGen docs (feature matrix, capability, optimization subsection).
Integration test support for offloading
tests/integration/defs/examples/test_visual_gen.py, tests/integration/test_lists/test-db/l0_b200.yml
_generate_wan_video() accepts extra_args for variant-specific CLI flags. test_wan_t2v_example_with_offloading() runs WAN example with --enable_offloading. Test list includes offloading test modules.
Unit tests for offloading core behavior
tests/unittest/_torch/visual_gen/test_offloading.py
Comprehensive PyTest suite validating CUDA-graph incompatibility, offload_context initialization, configured stage resolution/filtering, YAML loading, CPU/GPU allocation error context, packing order, alignment, staging/backing across groups, state_dict semantics, and view caching.
Wan 2.1 T2V offload integration tests
tests/unittest/_torch/visual_gen/test_wan21_t2v_offload.py
End-to-end offload coverage for Wan 2.1 1.3B: pipeline creation with OffloadConfig, offload init/execution assertions, baseline matching via cosine similarity (0.99 threshold), FP8 variant, and CUDA-graph incompatibility negative test.
Wan 2.1 pipeline correctness with offload
tests/unittest/_torch/visual_gen/test_wan21_t2v_pipeline.py
Adds enable_offload parameter to TRT-LLM pipeline loading and HF baseline comparison helpers. New test_cosine_similarity_with_offload() validates offload-enabled correctness.
Wan 2.2 two-stage T2V offload integration tests
tests/unittest/_torch/visual_gen/test_wan22_t2v_offload.py
End-to-end offload coverage for Wan 2.2 A14B: pipeline creation with OffloadConfig, offload init/execution assertions, baseline matching (0.99 cosine threshold), FP8 variant, and CUDA-graph incompatibility negative test.
Wan 2.2 pipeline correctness with offload
tests/unittest/_torch/visual_gen/test_wan22_t2v_pipeline.py
Adds enable_offload parameter to TRT-LLM pipeline loading and HF baseline comparison helpers. New test_cosine_similarity_with_offload() validates offload-enabled correctness.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

VisualGen

Suggested reviewers

  • chang-l
  • nv-guomingz
  • yuxianq
  • schetlur-nv
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The PR description provides a clear explanation of objectives, changes, test coverage, and performance results. It follows the template structure with sections for Description, Test Coverage, and a completed PR Checklist, though it lacks an explicit 'API changes' label statement.
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.
Title check ✅ Passed The title clearly and specifically summarizes the main feature addition: offloading support for visual_gen. It is concise, directly related to the core changeset, and provides meaningful context for code history scanning.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tensorrt_llm/_torch/visual_gen/offloading.py (1)

377-377: ⚡ Quick win

Add strict=True to zip() for safer iteration.

The zip() call pairs self.stages with self._stage_names, which are constructed together and should always have the same length. Adding strict=True will catch logic errors if they ever fall out of sync.

🔒 Proposed fix
-        for stage, group_name in zip(self.stages, self._stage_names):
+        for stage, group_name in zip(self.stages, self._stage_names, strict=True):

Based on learnings: In TensorRT-LLM, Python 3.10+ features are available throughout the codebase.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/visual_gen/offloading.py` at line 377, The loop uses
zip(self.stages, self._stage_names) which silently truncates if the two
iterables ever differ in length; update the iteration to zip(..., strict=True)
to enforce they stay matched and raise an error if they ever fall out of
sync—modify the loop where zip(self.stages, self._stage_names) is used (e.g., in
the offloading iteration block) to pass strict=True.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tensorrt_llm/_torch/visual_gen/offloading.py`:
- Line 377: The loop uses zip(self.stages, self._stage_names) which silently
truncates if the two iterables ever differ in length; update the iteration to
zip(..., strict=True) to enforce they stay matched and raise an error if they
ever fall out of sync—modify the loop where zip(self.stages, self._stage_names)
is used (e.g., in the offloading iteration block) to pass strict=True.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ce462aca-6de3-431b-810d-57c128a49b77

📥 Commits

Reviewing files that changed from the base of the PR and between 07770d9 and 0ad1878.

📒 Files selected for processing (8)
  • examples/visual_gen/README.md
  • examples/visual_gen/visual_gen_wan_t2v.py
  • tensorrt_llm/_torch/visual_gen/executor.py
  • tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan.py
  • tensorrt_llm/_torch/visual_gen/models/wan/transformer_wan.py
  • tensorrt_llm/_torch/visual_gen/offloading.py
  • tensorrt_llm/_torch/visual_gen/pipeline.py
  • tensorrt_llm/_torch/visual_gen/pipeline_loader.py

@NVShreyas

Copy link
Copy Markdown
Collaborator

Few initial thoughts -

  1. What is the main offloading method? From the code, I think its at model level. If so, the harness seems a bit overkill. Why can't we do model.to(device) before and after?
  2. the setup right now overrides the same GPU arena which could cause silent stale pointers if another group is accessed. Any code that touches a non-active group's parameter outside its own forward — cache extractor setup, state-dict export, CUDA-graph capture — silently sees or corrupts the wrong data.

Comment thread tensorrt_llm/_torch/visual_gen/executor.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/executor.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/executor.py Outdated
@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@NVShreyas

  1. What is the main offloading method? From the code, I think its at model level. If so, the harness seems a bit overkill. Why can't we do model.to(device) before and after?

I agree that model.to(device) before/after use is simpler and is a good baseline. The reason I didn’t start there is that it has a few downsides for this use case:

  • It copies each parameter/buffer separately, which can turn staging into many small H2D/D2H transfers instead of one large bandwidth-friendly transfer.
  • Moving back with .to("cpu") copies GPU weights back to CPU, even though inference weights are immutable and we can keep CPU as the source of truth.
  • It does not naturally give us a persistent packed pinned-memory CPU store. We can use pinned tensors in PyTorch, but once we want pinned CPU master weights plus large flattened H2D copies, we are already outside plain module-level .to().
  • It may create more allocator churn because module parameters are repeatedly rebound to newly allocated device storage.
  • Avoiding the D2H leg should cut the transfer work roughly in half for immutable weights, and packing the tensors should reduce copy launch overhead.

So the harness is intended to trade complexity for fewer transfers, pinned CPU backing storage, and larger contiguous copies. That said, I agree the design only makes sense if it avoids stale CUDA views and remains clearly scoped.

@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@NVShreyas

  1. the setup right now overrides the same GPU arena which could cause silent stale pointers if another group is accessed. Any code that touches a non-active group's parameter outside its own forward — cache extractor setup, state-dict export, CUDA-graph capture — silently sees or corrupts the wrong data.

This makes sense, I agree. The current invariant should be tightened so inactive/offloaded weights are either CPU-backed or fail loudly if accessed as CUDA weights. Silent stale CUDA views into a reused arena are not acceptable.

I’ll work on this next week. My current direction is to ensure only the active group is bound to the GPU arena, while inactive groups are rebound to CPU storage or otherwise guarded so accidental use errors out. Happy to take pointers on whether you’d prefer CPU-backed inactive params or an explicit sentinel/error tensors.

@rahul-steiger-nv
rahul-steiger-nv force-pushed the visual_gen_offloading branch from fbc06fd to 92e09f8 Compare May 18, 2026 13:54

@NVShreyas NVShreyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall the approach is good and I'm okay with not using module.to(device) based on the explanation you provided. Lets make sure the code is architected in a maintainable and readable manner (including docstrings). It also should be tested with all single GPU features like quantization, caching, torch compile, etc (or raise NotImplementedError explicitly). Let's leave multi-gpu offloading as a future enhancement for now.

Comment thread tensorrt_llm/_torch/visual_gen/models/wan/transformer_wan.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/models/wan/transformer_wan.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/models/wan/transformer_wan.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/offloading.py Outdated
@rahul-steiger-nv
rahul-steiger-nv force-pushed the visual_gen_offloading branch 2 times, most recently from 05f439e to 9c77ea6 Compare May 19, 2026 11:24
@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@coderabbitai help

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai rate limit to show your current review rate limit status.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
  • @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
  • @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve merge conflict to automatically resolve merge conflicts.
  • @coderabbitai autofix to automatically fix issues identified in unresolved review comments.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Status, support, documentation and community

  • Visit our status page to check the current availability of CodeRabbit.
  • Create a ticket on our support page for assistance with any issues or questions.
  • Visit our documentation site for detailed information on how to use CodeRabbit.
  • Join our Discord community to connect with other users and get help from the community.
  • Follow us on X/Twitter for updates and announcements.

@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@coderabbitai run pre-merge checks

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Running Pre-merge checks. Results will be posted in the summary comment once complete.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
tensorrt_llm/_torch/visual_gen/offloading.py (1)

410-415: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Cached GPU views still leave stale arena-backed tensors reachable.

layout.gpu_views is created once and then rebound in and out of modules, but any Parameter/buffer reference captured while a group is active still points at self.gpu_arena after _rebind_to_cpu(). Once the next group stages, that old object silently reads unrelated weights instead of failing, so the stale-CUDA-view hazard called out earlier is still present.

Please either recreate/poison GPU-backed view objects on each activation or add a hard guard that prevents arena-backed tensors from escaping the active call scope.

Also applies to: 425-466

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/visual_gen/offloading.py` around lines 410 - 415, Cached
GPU views in layout.gpu_views let Parameter/buffer objects keep pointers into
self.gpu_arena after _rebind_to_cpu(), causing stale-arena reads; fix by
ensuring GPU-backed view objects are not reusable across activations: either (A)
recreate GPU views on every activation and/or clear/poison old layout.gpu_views
when deactivating, or (B) add a hard guard in _rebind_to_cpu() (and activation
entry) that walks module parameters and buffers and replaces any tensor whose
storage points at self.gpu_arena with a safe CPU-backed tensor or raises an
error if it would escape scope; implement the change around calls to
_make_views, layout.gpu_views, and _rebind_to_cpu so no arena-backed tensors can
remain reachable after deactivation.
🧹 Nitpick comments (2)
tensorrt_llm/_torch/visual_gen/config.py (1)

351-351: ⚡ Quick win

Add Pydantic field description for the new user-facing flag.

PipelineConfig is user-facing, so this new field should use PydanticField with a description for schema/help consistency.

♻️ Proposed fix
-    enable_cuda_memory_logging: bool = False
+    enable_cuda_memory_logging: bool = PydanticField(
+        False,
+        description="Enable per-request CUDA peak memory logging in diffusion executor.",
+    )

As per coding guidelines: "Add descriptions to all user-facing Pydantic fields via Field(description="...") rather than using comments".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/visual_gen/config.py` at line 351, The new user-facing
flag enable_cuda_memory_logging is declared as a bare bool; update
PipelineConfig to use PydanticField with a description (e.g.
enable_cuda_memory_logging: bool = PydanticField(False, description="Enable
detailed CUDA memory logging for debugging and profiling") ) so it appears in
the generated schema/help; also add or ensure the PydanticField import is
present and follow the same description style used by other PipelineConfig
fields.
tests/unittest/_torch/visual_gen/test_offloading.py (1)

1-268: No QA integration test-list update is needed for this change set.

This PR scope adds unit tests under tests/unittest/..., so updating tests/integration/test_lists/qa/* is unnecessary.

As per coding guidelines, “If the PR only touches unittest/ or narrow unit scope, say explicitly whether QA list updates are unnecessary or optional.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/visual_gen/test_offloading.py` around lines 1 - 268,
Add an explicit note to the PR description or commit message stating that this
change only adds unit tests (test_offloading.py) and therefore no QA integration
test-list updates are required; include the exact sentence "No QA integration
test-list update is needed for this change set." so reviewers and release
tooling can detect it, and update any PR checklist/description fields that
record whether integration QA lists were modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tensorrt_llm/_torch/visual_gen/executor.py`:
- Around line 349-355: Change the broad except Exception in the CUDA-memory
helper try/except blocks to catch RuntimeError only: for the block calling
torch.cuda.reset_peak_memory_stats(self.device_id) and the similar block around
lines handling CUDA memory (the second try/except referenced in the comment),
replace "except Exception as e" with "except RuntimeError as e" so only PyTorch
CUDA runtime errors are caught and other exceptions are not masked; keep the
same logger.warning message and error variable name.

In `@tensorrt_llm/_torch/visual_gen/pipeline_loader.py`:
- Around line 280-286: The code is using pipeline.default_offload_stages() which
hard-codes the full default set; instead fetch and pass the configured offload
stages via the same resolution path used by runtime init (the helper used by
initialize_offload_pipeline()). Replace the call to
pipeline.default_offload_stages() inside the
pipeline._filter_available_offload_stages(...) call with the configured-stage
resolver (e.g., the pipeline method/attribute used by
initialize_offload_pipeline(), such as pipeline.offload_stages() or
pipeline._resolve_offload_stages(...)) so the collected parts mirror the runtime
configuration and avoid materializing modules that should not be staged.

In `@tensorrt_llm/_torch/visual_gen/pipeline.py`:
- Around line 366-370: The method offload_context currently returns
nullcontext() when self._offload_pipeline is None, which silently masks
misordered initialization; instead detect when offloading is intended (enable
True) but _offload_pipeline is uninitialized and raise a clear exception
indicating offload pipeline not initialized (refer to offload_context and
self._offload_pipeline), instruct callers to run initialize_offload_pipeline()
first; apply the same explicit guard/exception pattern to the other places that
currently check self._offload_pipeline (the subsequent blocks surrounding
offload setup/usage) so they fail fast rather than silently falling back to CPU
behavior.

In `@tests/unittest/_torch/visual_gen/test_offloading.py`:
- Around line 153-156: The regex in the pytest.raises assertion uses
"group.weight" where the dot is a regex metacharacter; update the match argument
in the pytest.raises call in tests/unittest/_torch/visual_gen/test_offloading.py
(the block using pytest.raises(RuntimeError, match="group.weight")) to escape
the dot (e.g., "group\.weight" or use re.escape on the literal) so the assertion
matches the literal field name rather than any character.

---

Duplicate comments:
In `@tensorrt_llm/_torch/visual_gen/offloading.py`:
- Around line 410-415: Cached GPU views in layout.gpu_views let Parameter/buffer
objects keep pointers into self.gpu_arena after _rebind_to_cpu(), causing
stale-arena reads; fix by ensuring GPU-backed view objects are not reusable
across activations: either (A) recreate GPU views on every activation and/or
clear/poison old layout.gpu_views when deactivating, or (B) add a hard guard in
_rebind_to_cpu() (and activation entry) that walks module parameters and buffers
and replaces any tensor whose storage points at self.gpu_arena with a safe
CPU-backed tensor or raises an error if it would escape scope; implement the
change around calls to _make_views, layout.gpu_views, and _rebind_to_cpu so no
arena-backed tensors can remain reachable after deactivation.

---

Nitpick comments:
In `@tensorrt_llm/_torch/visual_gen/config.py`:
- Line 351: The new user-facing flag enable_cuda_memory_logging is declared as a
bare bool; update PipelineConfig to use PydanticField with a description (e.g.
enable_cuda_memory_logging: bool = PydanticField(False, description="Enable
detailed CUDA memory logging for debugging and profiling") ) so it appears in
the generated schema/help; also add or ensure the PydanticField import is
present and follow the same description style used by other PipelineConfig
fields.

In `@tests/unittest/_torch/visual_gen/test_offloading.py`:
- Around line 1-268: Add an explicit note to the PR description or commit
message stating that this change only adds unit tests (test_offloading.py) and
therefore no QA integration test-list updates are required; include the exact
sentence "No QA integration test-list update is needed for this change set." so
reviewers and release tooling can detect it, and update any PR
checklist/description fields that record whether integration QA lists were
modified.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 00865218-f5b0-46ae-9ce1-912e0f6d4817

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad1878 and 32adbfc.

⛔ Files ignored due to path filters (2)
  • examples/visual_gen/output_wan_fp8_offload.avi is excluded by !**/*.avi
  • examples/visual_gen/output_wan_fp8_offload_quality.avi is excluded by !**/*.avi
📒 Files selected for processing (9)
  • examples/visual_gen/README.md
  • examples/visual_gen/visual_gen_wan_t2v.py
  • tensorrt_llm/_torch/visual_gen/config.py
  • tensorrt_llm/_torch/visual_gen/executor.py
  • tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan.py
  • tensorrt_llm/_torch/visual_gen/offloading.py
  • tensorrt_llm/_torch/visual_gen/pipeline.py
  • tensorrt_llm/_torch/visual_gen/pipeline_loader.py
  • tests/unittest/_torch/visual_gen/test_offloading.py
✅ Files skipped from review due to trivial changes (1)
  • examples/visual_gen/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • examples/visual_gen/visual_gen_wan_t2v.py

Comment thread tensorrt_llm/_torch/visual_gen/executor.py
Comment thread tensorrt_llm/_torch/visual_gen/pipeline_loader.py Outdated
Comment thread tensorrt_llm/_torch/visual_gen/pipeline.py Outdated
Comment thread tests/unittest/_torch/visual_gen/test_offloading.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/visual_gen/executor.py (1)

309-323: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prevent stale CUDA peak-memory logs on early failures.

If request handling fails before the reset call, the exception-path log can report peak memory from a previous request. Reset at request start (or gate logging on a per-request reset flag).

Proposed fix
 def process_request(self, req: DiffusionRequest):
     """Process a single request."""
     log_cuda_memory = self._cuda_memory_logging_enabled()
+    did_reset_peak_stats = False
     try:
+        if log_cuda_memory:
+            self._reset_cuda_peak_memory_stats()
+            did_reset_peak_stats = True
         self._merge_defaults(req)
         cache_key = self.pipeline.warmup_cache_key(
             req.params.height, req.params.width, num_frames=req.params.num_frames
         )
@@
-            if log_cuda_memory:
-                self._reset_cuda_peak_memory_stats()
             output = self.pipeline.infer(req)
-            if log_cuda_memory:
+            if did_reset_peak_stats:
                 self._log_cuda_peak_memory(req.request_id)
@@
-            if log_cuda_memory:
+            if did_reset_peak_stats:
                 self._log_cuda_peak_memory(req.request_id)

Also applies to: 325-326, 330-331

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tensorrt_llm/_torch/visual_gen/executor.py` around lines 309 - 323, Move the
per-request CUDA peak-memory reset to the start of request handling so stale
peak-memory values cannot be logged on early failures: call
self._reset_cuda_peak_memory_stats() immediately after determining
log_cuda_memory with self._cuda_memory_logging_enabled() (before calling
self._merge_defaults or any code that can raise), and ensure the same pattern is
applied for the other CUDA-logging sites in this file (the blocks that reference
self._reset_cuda_peak_memory_stats() later) so logging is gated by a per-request
reset rather than relying on post-success cleanup.
♻️ Duplicate comments (1)
tests/unittest/_torch/visual_gen/test_offloading.py (1)

153-156: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape the dot in the pytest.raises(match=...) pattern.

group.weight is interpreted as regex (. = any char), so this assertion can pass on unintended messages.

Proposed fix
     with pytest.raises(
         RuntimeError,
-        match="Failed to copy offload tensor 'group.weight'",
+        match=r"Failed to copy offload tensor 'group\.weight'",
     ) as exc_info:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/visual_gen/test_offloading.py` around lines 153 - 156,
The regex in the pytest.raises call (pytest.raises(..., match="Failed to copy
offload tensor 'group.weight'")) treats the dot in "group.weight" as a wildcard;
update the match to escape the dot so it matches a literal period (e.g., use a
raw string with the dot escaped or use re.escape on "group.weight") in the
pytest.raises call in test_offloading.py to ensure the assertion only matches
the exact message.
🧹 Nitpick comments (1)
tests/unittest/_torch/visual_gen/test_offloading.py (1)

1-268: QA list update scope is unnecessary for this PR surface.

These changes are confined to tests/unittest/...; no tests/integration/test_lists/qa/* update is needed in this PR.

As per coding guidelines, "If the PR only touches unittest/ or narrow unit scope, say explicitly whether QA list updates are unnecessary or optional."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/_torch/visual_gen/test_offloading.py` around lines 1 - 268,
The PR touches only unit test files under tests/unittest and therefore does not
require updating the QA lists; update the PR description (or add a short
commit/PR note) to explicitly state that "QA list updates are unnecessary"
because the changes are confined to tests/unittest (per the guideline to state
whether QA list updates are unnecessary or optional), referencing this scope and
the fact no tests/integration/test_lists/qa/* files were modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tensorrt_llm/_torch/visual_gen/executor.py`:
- Around line 309-323: Move the per-request CUDA peak-memory reset to the start
of request handling so stale peak-memory values cannot be logged on early
failures: call self._reset_cuda_peak_memory_stats() immediately after
determining log_cuda_memory with self._cuda_memory_logging_enabled() (before
calling self._merge_defaults or any code that can raise), and ensure the same
pattern is applied for the other CUDA-logging sites in this file (the blocks
that reference self._reset_cuda_peak_memory_stats() later) so logging is gated
by a per-request reset rather than relying on post-success cleanup.

---

Duplicate comments:
In `@tests/unittest/_torch/visual_gen/test_offloading.py`:
- Around line 153-156: The regex in the pytest.raises call (pytest.raises(...,
match="Failed to copy offload tensor 'group.weight'")) treats the dot in
"group.weight" as a wildcard; update the match to escape the dot so it matches a
literal period (e.g., use a raw string with the dot escaped or use re.escape on
"group.weight") in the pytest.raises call in test_offloading.py to ensure the
assertion only matches the exact message.

---

Nitpick comments:
In `@tests/unittest/_torch/visual_gen/test_offloading.py`:
- Around line 1-268: The PR touches only unit test files under tests/unittest
and therefore does not require updating the QA lists; update the PR description
(or add a short commit/PR note) to explicitly state that "QA list updates are
unnecessary" because the changes are confined to tests/unittest (per the
guideline to state whether QA list updates are unnecessary or optional),
referencing this scope and the fact no tests/integration/test_lists/qa/* files
were modified.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1e658a24-a4fc-4f38-bedc-a2b2872a2bd5

📥 Commits

Reviewing files that changed from the base of the PR and between 989671b and 32adbfc.

⛔ Files ignored due to path filters (2)
  • examples/visual_gen/output_wan_fp8_offload.avi is excluded by !**/*.avi
  • examples/visual_gen/output_wan_fp8_offload_quality.avi is excluded by !**/*.avi
📒 Files selected for processing (9)
  • examples/visual_gen/README.md
  • examples/visual_gen/visual_gen_wan_t2v.py
  • tensorrt_llm/_torch/visual_gen/config.py
  • tensorrt_llm/_torch/visual_gen/executor.py
  • tensorrt_llm/_torch/visual_gen/models/wan/pipeline_wan.py
  • tensorrt_llm/_torch/visual_gen/offloading.py
  • tensorrt_llm/_torch/visual_gen/pipeline.py
  • tensorrt_llm/_torch/visual_gen/pipeline_loader.py
  • tests/unittest/_torch/visual_gen/test_offloading.py

@rahul-steiger-nv
rahul-steiger-nv requested a review from a team as a code owner July 29, 2026 18:10
@ishovkun

Copy link
Copy Markdown
Contributor

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62805 [ run ] triggered by Bot. Commit: 8d180d0 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62805 [ run ] completed with state FAILURE. Commit: 8d180d0
/LLM/main/L0_MergeRequest_PR pipeline #50932 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@ishovkun

Copy link
Copy Markdown
Contributor

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62849 [ run ] triggered by Bot. Commit: 8d180d0 Link to invocation

@ishovkun

Copy link
Copy Markdown
Contributor

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #62849 [ run ] completed with state FAILURE. Commit: 8d180d0
/LLM/main/L0_MergeRequest_PR pipeline #50973 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Chang Liu <9713593+chang-l@users.noreply.github.com>
Signed-off-by: Chang Liu <9713593+chang-l@users.noreply.github.com>
@chang-l

chang-l commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@ishovkun

Copy link
Copy Markdown
Contributor

Maybe already obsolete, but:
One of the failures IS this PR's fault

test_ltx2_pipeline.py::TestLTX2TwoStageLoRAHelpers::test_run_warmup_precaptures_both_topologies is a genuine regression. Verified empirically, not inferred:

has _device attr?  False
AttributeError: 'Dummy' object has no attribute 'device'

The chain:

  1. The test builds the pipeline via object.__new__ (line 1283), deliberately skipping __init__
  2. It calls _run_warmup, which at pipeline_ltx2.py:682 does torch.zeros(..., device=self.device)
  3. This PR changed BasePipeline.device from return self.transformer.devicereturn self._device
  4. _device is assigned only in BasePipeline.__init__ (pipeline.py:129) — which never ran

A/B confirms it:

result
old impl self.transformer.device cudapasses
new impl, _device unset AttributeError
new impl, _device set cuda → passes

Fix

The semantic change is correct — under offloading the transformer can sit on CPU, so self.transformer.device would report the wrong device. So the test should adapt, not the property revert. One line in test_ltx2_pipeline.py, mirroring what __init__ does:

pipeline = object.__new__(ltx2_two_stages.LTX2TwoStagesPipeline)
pipeline._device = torch.device("cuda")   # __init__ is bypassed; device property reads this

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63078 [ run ] triggered by Bot. Commit: 7617d4e Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63078 [ run ] completed with state FAILURE. Commit: 7617d4e
/LLM/main/L0_MergeRequest_PR pipeline #51172 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@chang-l

chang-l commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63134 [ run ] triggered by Bot. Commit: 7617d4e Link to invocation

@chang-l

chang-l commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

@rahul-steiger-nv Please address the deterministic LTX2 unit-test regression identified in #14095 (comment) before another CI retry. The PR correctly changes BasePipeline.device to use _device, but test_run_warmup_precaptures_both_topologies constructs the pipeline with object.__new__ and therefore must initialize pipeline._device = torch.device("cuda"). L0 build 51172 reproduced the test_ltx2_pipeline.py wrapper failure on current head 7617d4e. A rerun is already in progress, but I have paused automated babysitting to prevent further retries until a fix is pushed. Please ping us after updating the PR so monitoring can resume.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63134 [ run ] completed with state FAILURE. Commit: 7617d4e
/LLM/main/L0_MergeRequest_PR pipeline #51222 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

Signed-off-by: Rahul Steiger <rsteiger@nvidia.com>
@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

@chang-l @ishovkun Thank you for reviewing the PR. I’ve addressed the requested changes and will rerun CI/CD.

@rahul-steiger-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #63301 [ run ] triggered by Bot. Commit: c488467 Link to invocation

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.