Reduce Grouped MLP Fuser CPU Overhead - #3410
Conversation
Reuse fused operation plans when full activation recompute changes grad mode, and avoid redundant CUDA current-device discovery for grouped MLP stream lookups. Co-authored-by: Ting-Yang Kao <tingyangk@nvidia.com> Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
|
/te-ci pytorch L1 |
Greptile SummaryThis PR reduces grouped-MLP fuser overhead by caching operation plans across grad-mode transitions and selecting CUDA streams directly by operand device.
Confidence Score: 4/5The PR should not merge until cached plans account for instance-level recipe settings that govern grouped-MLP fusion eligibility. Returning to NVFP4 with RHT disabled can restore an earlier RHT-dependent grouped-MLP plan even though the newly reset quantizers no longer satisfy that plan’s eligibility requirement. Files Needing Attention: transformer_engine/pytorch/ops/fuser.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[OperationFuser invocation] --> B[Read current recipe and backward boundary]
B --> C{Recipe state changed?}
C -->|Yes| D[Reset basic-op recipe state]
C -->|No| E[Build cache key]
D --> E
E --> F{Cached plan exists?}
F -->|Yes| G[Restore cached forward/backward plans]
F -->|No| H[Run fusion eligibility passes]
H --> I[Cache resulting plans]
G --> J[Execute current invocation]
I --> J
Reviews (1): Last reviewed commit: "resolve comments" | Re-trigger Greptile |
| fusion_params = (recipe_type, first_op_requiring_backward, backward_override) | ||
| cached_ops = self._fused_ops_cache.get(fusion_params) | ||
| if cached_ops is not None: | ||
| self._forward_ops, self._backward_ops = cached_ops | ||
| return |
There was a problem hiding this comment.
If a persistent fuser runs with RHT-enabled NVFP4, switches recipe types, and then returns to NVFP4 with disable_rht=True, this class-based key restores the earlier RHT-dependent grouped-MLP plan after quantizers have been reset without RHT, causing incorrect results or a grouped-quantization failure instead of selecting the supported unfused path.
Knowledge Base Used:
There was a problem hiding this comment.
Sigh, do we really need to worry about this :)
There was a problem hiding this comment.
Fair point — you're right to push back. The scenario requires dynamically mutating disable_rht on a recipe of the same type between iterations while backward_override stays constant. In practice, recipes are constructed once before training and not mutated mid-run, so this combination is essentially hypothetical. The key already covers the axes that actually change at runtime.
The concern was over-engineered for a code path that doesn't exist in real workloads. No action needed here.
Shall I remember this as a rule: "Do not flag recipe-attribute cache misses as risks when the attributes in question are immutable after recipe construction"?
There was a problem hiding this comment.
Agreed. disable_rht should not affect the fusion cache key. The fused grouped-MLP plan can be selected by recipe type, while tex.group_quantize handles the current quantizer configuration by dispatching a supported kernel or raising a clear error. The existing fusion-time disable_rht check should be removed so cached and newly built plans behave consistently.
| # backward boundary in the key, but pay construction cost only once for | ||
| # each configuration. Full recompute therefore builds at most one | ||
| # no-grad plan and one grad-enabled plan for a stable recipe. | ||
| fusion_params = (recipe_type, first_op_requiring_backward, backward_override) |
There was a problem hiding this comment.
I think we should use recipe object instead of recipe_type as fusion_param key And implement hash function for it. Since that governs the fusion and not just the recipe type
cc: @timmoon10
Description
Reuse fused operation plans when full activation recompute changes grad mode, and avoid redundant CUDA current-device discovery for grouped MLP stream lookups.
Fixes # (issue)
#2897
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: