-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[TRTLLM-14541][fix] VisualGen: byte-identical tune-run vs cache-load on multi-GPU #16782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a5b6274
7e4410f
e9b1b62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -72,6 +72,8 @@ def __init__( | |||||
| ): | ||||||
| self.config = config | ||||||
| self.enabled = config.use_cuda_graph | ||||||
| # When False, uncaptured keys run eagerly instead of triggering capture. | ||||||
| self.capture_enabled = True | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
"enabled" sounds like a "static status/config" to me. |
||||||
| self._shared_pool = shared_pool | ||||||
| self._extra_key_fns: Dict[str, ExtraKeyFn] = {} | ||||||
|
|
||||||
|
|
@@ -194,6 +196,8 @@ def wrapper(*args, **kwargs): | |||||
| key = self.get_graph_key(*args, **kwargs) | ||||||
|
|
||||||
| if key not in self.graphs: | ||||||
| if not self.capture_enabled: | ||||||
| return fn(*args, **kwargs) | ||||||
| self.capture(key, fn, args, kwargs) | ||||||
| return self.replay(key, args, kwargs) | ||||||
| else: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import contextlib | ||||||
| import itertools | ||||||
| import os | ||||||
| import time | ||||||
|
|
@@ -206,6 +207,18 @@ def _setup_cuda_graphs(self): | |||||
| model.forward = runner.wrap(model.forward) | ||||||
| self._cuda_graph_runners[name] = runner | ||||||
|
|
||||||
| @contextlib.contextmanager | ||||||
| def no_cuda_graph_capture(self): | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| """Run wrapped forwards eagerly instead of capturing new CUDA graphs.""" | ||||||
| prev = {name: r.capture_enabled for name, r in self._cuda_graph_runners.items()} | ||||||
| for r in self._cuda_graph_runners.values(): | ||||||
| r.capture_enabled = False | ||||||
| try: | ||||||
| yield | ||||||
| finally: | ||||||
| for name, r in self._cuda_graph_runners.items(): | ||||||
| r.capture_enabled = prev[name] | ||||||
|
|
||||||
| @property | ||||||
| def rank(self): | ||||||
| return dist.get_rank() if dist.is_initialized() else 0 | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,19 +22,40 @@ | |
| import torch.distributed as dist | ||
|
|
||
| from tensorrt_llm._torch.autotuner import autotune | ||
| from tensorrt_llm._torch.distributed.communicator import Distributed, TorchDist | ||
| from tensorrt_llm._torch.models.modeling_utils import MetaInitMode | ||
| from tensorrt_llm._torch.visual_gen.cute_dsl_kernels.blackwell.video_sparse_attention import ( | ||
| CUTE_AVAILABLE, | ||
| ) | ||
| from tensorrt_llm.llmapi.utils import download_hf_model | ||
| from tensorrt_llm.logger import logger | ||
| from tensorrt_llm.mapping import Mapping | ||
| from tensorrt_llm.visual_gen.args import VisualGenArgs | ||
|
|
||
| from .config import DiffusionPipelineConfig | ||
| from .mapping import VisualGenMapping | ||
| from .models import AutoPipeline | ||
| from .pipeline_registry import PIPELINE_REGISTRY, PipelineComponent | ||
|
|
||
|
|
||
| class _VisualGenAutotuneDist(TorchDist): | ||
| """Autotuner communicator whose collective spans the whole world. | ||
|
|
||
| VisualGen's device mesh has no TP axis over the tuned GEMMs (its parallelism | ||
| is on cfg/ulysses), so the mesh's tp group would gather a single rank. Gather | ||
| over the default world group instead, since every rank runs the transformer. | ||
| """ | ||
|
|
||
| def __init__(self, mapping: Mapping): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need TorchDist init here? |
||
| Distributed.__init__(self, mapping) | ||
| assert dist.is_initialized() | ||
|
|
||
| def tp_cp_allgather(self, obj: object) -> list[object]: | ||
| gathered: list[object] = [None] * dist.get_world_size() | ||
| dist.all_gather_object(gathered, obj) | ||
| return gathered | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| from .models import BasePipeline | ||
|
|
||
|
|
@@ -305,10 +326,32 @@ def load( | |
|
|
||
| if not skip_warmup: | ||
| if config.torch_compile.enable_autotune: | ||
| with autotune( | ||
| cache_path=os.environ.get("TLLM_AUTOTUNER_CACHE_PATH"), | ||
| skip_dynamic_tuning_buckets=True, | ||
| ): | ||
| cache_path = os.environ.get("TLLM_AUTOTUNER_CACHE_PATH") | ||
| post_tune_merge_dist = None | ||
| if dist.is_initialized() and dist.get_world_size() > 1: | ||
| amap = config.visual_gen_mapping.to_autotuner_mapping() | ||
| post_tune_merge_dist = _VisualGenAutotuneDist(amap) | ||
|
|
||
| if post_tune_merge_dist is None: | ||
| # Single rank: no cross-rank merge, so tune and capture in one | ||
| # pass (the graph runner captures only after its own warmup | ||
| # steps have finalized the tactic). | ||
| with autotune(cache_path=cache_path, skip_dynamic_tuning_buckets=True): | ||
| pipeline.warmup() | ||
| else: | ||
| # Multi rank: capture graphs only AFTER tactics are merged | ||
| # across ranks, otherwise each rank bakes its own pre-merge | ||
| # tactic. Phase 1 tunes with capture off; the autotuner merges | ||
| # at autotune() exit; phase 2 captures the merged tactics. | ||
| with ( | ||
| pipeline.no_cuda_graph_capture(), | ||
| autotune( | ||
| cache_path=cache_path, | ||
| skip_dynamic_tuning_buckets=True, | ||
| post_tune_merge_dist=post_tune_merge_dist, | ||
| ), | ||
| ): | ||
| pipeline.warmup() | ||
|
Comment on lines
+342
to
+354
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the impact to pipeline start up time? If the any steps in the Do we need this if CUDA graph not enabled? |
||
| pipeline.warmup() | ||
| else: | ||
| pipeline.warmup() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import enum | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add the required NVIDIA copyright header/year. This modified file begins with an import and has no copyright header. Add the repository-standard NVIDIA header with the 2026 year. As per coding guidelines, “Add the NVIDIA copyright header to all new files and update the copyright year on modified files.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||
| import itertools | ||||||
| import json | ||||||
| import math | ||||||
|
|
@@ -1265,3 +1266,90 @@ def _cublas_call(): | |||||
| f"CuteDSL heuristic kernel ({heuristic_us:.2f} us) is " | ||||||
| f">{cublas_tolerance:.2f}x slower than cuBLAS NVFP4 " | ||||||
| f"({cublas_us:.2f} us) for M={m}, N={n}, K={k}") | ||||||
|
|
||||||
|
|
||||||
| def test_post_tune_merge_tactics_min_time_and_subset_kept(): | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to run multi-GPU verification for this part. |
||||||
| tuner = autotuner.AutoTuner() | ||||||
| tuner.mapping = Mapping(world_size=2, rank=0, tp_size=2) | ||||||
|
|
||||||
| r0 = { | ||||||
| ("gemm", "X"): (0, ("cutlass", 10), 1.0), | ||||||
| ("q", "A"): (0, ("trtllm", ), 1.0) | ||||||
| } | ||||||
| r1 = { | ||||||
| ("gemm", "X"): (0, ("cublaslt", 0), 0.5), | ||||||
| ("vae", "B"): (0, ("cutlass", 2), 1.0) | ||||||
| } | ||||||
|
|
||||||
| class _FakeDist: | ||||||
|
|
||||||
| def tp_cp_allgather(self, obj): | ||||||
| return [r0, r1] | ||||||
|
|
||||||
| tuner._dist = _FakeDist() | ||||||
| tuner.profiling_cache.cache = dict(r0) | ||||||
| tuner.post_tune_merge_tactics() | ||||||
|
|
||||||
| cache = tuner.profiling_cache.cache | ||||||
| assert cache[("gemm", "X")] == (0, ("cublaslt", 0), 0.5) | ||||||
| assert cache[("q", "A")] == r0[("q", "A")] | ||||||
| assert cache[("vae", "B")] == r1[("vae", "B")] | ||||||
|
|
||||||
|
|
||||||
| def test_post_tune_merge_tactics_single_rank_noop(): | ||||||
| tuner = autotuner.AutoTuner() | ||||||
| tuner.mapping = Mapping(world_size=1, rank=0, tp_size=1) | ||||||
| tuner._dist = None | ||||||
| original = {("gemm", "X"): (0, ("cutlass", 10), 1.0)} | ||||||
| tuner.profiling_cache.cache = dict(original) | ||||||
| tuner.post_tune_merge_tactics() | ||||||
| assert tuner.profiling_cache.cache == original | ||||||
|
|
||||||
|
|
||||||
| def test_profiling_cache_enum_tactic_roundtrip(tmp_path): | ||||||
| # An enum tactic must survive save -> load: repr("<Enum.X: v>") isn't | ||||||
| # ast.literal_eval-parsable, so the cache serializes tactic.value instead. | ||||||
| class _Tac(enum.IntEnum): | ||||||
| TRTLLM = -1 | ||||||
|
|
||||||
| key = ("op::x", "Runner", "(1,)") | ||||||
| src = autotuner.AutoTuner().profiling_cache | ||||||
| src.cache[key] = (0, _Tac.TRTLLM, 1.0) | ||||||
| path = str(tmp_path / "cache.json") | ||||||
| src.save_cache(path, rank=0) # must not raise | ||||||
|
|
||||||
| dst = autotuner.AutoTuner().profiling_cache | ||||||
| dst.load_cache(path, rank=0) | ||||||
| assert dst.cache[key][1] == _Tac.TRTLLM # IntEnum compares equal to -1 | ||||||
|
|
||||||
|
|
||||||
| def test_autotune_post_tune_merge_before_save(tmp_path): | ||||||
| # autotune(post_tune_merge_dist=...) must merge across ranks and persist the | ||||||
| # merged winner, then restore the singleton's prior distributed state. | ||||||
| tuner = AutoTuner.get() | ||||||
| tuner.profiling_cache.cache.clear() | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Reset the complete singleton profiling-cache state. Clearing only Proposed fix- tuner.profiling_cache.cache.clear()
+ tuner.clear_cache()Based on supplied autotuner context, 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| r0 = {("gemm", "X"): (0, ("cutlass", 10), 1.0)} | ||||||
| r1 = {("gemm", "X"): (0, ("cublaslt", 0), 0.5)} | ||||||
| tuner.profiling_cache.cache = dict(r0) | ||||||
|
|
||||||
| class _FakeDist: | ||||||
| mapping = Mapping(world_size=2, rank=0, tp_size=2) | ||||||
|
|
||||||
| def tp_cp_allgather(self, obj): | ||||||
| return [r0, r1] | ||||||
|
|
||||||
| prev_mapping, prev_dist = tuner.mapping, tuner._dist | ||||||
| path = str(tmp_path / "cache.json") | ||||||
| with autotune(cache_path=path, post_tune_merge_dist=_FakeDist()): | ||||||
| pass | ||||||
|
|
||||||
| # Merged in memory (fastest tactic won) and persisted before the context | ||||||
| # exits (the saved file carries the merged winner). | ||||||
| assert tuner.profiling_cache.cache[("gemm", "X")] == (0, ("cublaslt", 0), | ||||||
| 0.5) | ||||||
| persisted = autotuner.AutoTuner().profiling_cache | ||||||
| persisted.load_cache(path, rank=0) | ||||||
| assert persisted.cache[("gemm", "X")][1] == ("cublaslt", 0) | ||||||
| # The temporary full-world distributed state was restored. | ||||||
| assert tuner.mapping is prev_mapping | ||||||
| assert tuner._dist is prev_dist | ||||||
Uh oh!
There was an error while loading. Please reload this page.