[https://nvbugs/6272666][fix] Added a GPU-count-aware post-processing block in pytorch_model_config.py that… - #15028
Conversation
…llelism Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
|
Too many files changed? Review this PR in Change Stack to see how the pieces fit before you dive in. 📝 WalkthroughWalkthroughThe PR relocates Minimax-M2.5 FP8 MoE configuration logic in the test model configuration file. Pattern-based routing that previously matched specific input/output dimensions is removed, and replaced with a post-pattern conditional override that applies MoE expert parallel sizing and attention data-parallel settings when the PyTorch backend runs the model on 8 GPUs. ChangesMinimax-M2.5 FP8 Configuration Migration
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/defs/perf/pytorch_model_config.py (1)
604-605: 💤 Low valueConsider using
setdefault()for consistency.The code directly assigns to
base_configkeys, which will unconditionally overwrite any values set by earlier pattern_configs. The DeepSeek FP8 block immediately above (lines 592-593) usessetdefault()to avoid clobbering existing config.If this post-pattern override is intended to take precedence regardless of prior config, the current approach is correct. However, if you want to preserve values already set by pattern_configs, use
setdefault().♻️ Alternative using setdefault to preserve existing config
- base_config['moe_expert_parallel_size'] = 8 - base_config['enable_attention_dp'] = True + base_config.setdefault('moe_expert_parallel_size', 8) + base_config.setdefault('enable_attention_dp', True)This would only set the values if they haven't already been configured by a pattern_configs entry.
🤖 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/integration/defs/perf/pytorch_model_config.py` around lines 604 - 605, Change the unconditional assignments to preserve prior pattern_configs by using base_config.setdefault('moe_expert_parallel_size', 8) and base_config.setdefault('enable_attention_dp', True) instead of directly assigning to base_config['moe_expert_parallel_size'] and base_config['enable_attention_dp']; this ensures existing values configured earlier are not overwritten by these post-pattern defaults while keeping the same defaults if keys are absent.
🤖 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 `@tests/integration/defs/perf/pytorch_model_config.py`:
- Line 603: The current substring check using model_label.lower() and 'gpus:8'
can match unintended labels like "gpus:80"; update the condition that decides
the 8-GPU config to perform an exact segment match instead of a raw
substring—e.g., normalize model_label, then either split the label on delimiters
(spaces, commas, slashes, colons) and compare tokens for the exact token
"gpus:8", or use a regex with anchors/boundaries to match
r'(?<!\d)gpus:8(?!\d)'; change the conditional that references model_label to
use this precise match so only true 8-GPU labels select the MiniMax-M2.5 FP8
configuration.
---
Nitpick comments:
In `@tests/integration/defs/perf/pytorch_model_config.py`:
- Around line 604-605: Change the unconditional assignments to preserve prior
pattern_configs by using base_config.setdefault('moe_expert_parallel_size', 8)
and base_config.setdefault('enable_attention_dp', True) instead of directly
assigning to base_config['moe_expert_parallel_size'] and
base_config['enable_attention_dp']; this ensures existing values configured
earlier are not overwritten by these post-pattern defaults while keeping the
same defaults if keys are absent.
🪄 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: 9b6b2a52-ef55-4b38-b93d-9c24016a8ded
📒 Files selected for processing (1)
tests/integration/defs/perf/pytorch_model_config.py
| # passes only --tp=8, so moe_expert_parallel_size must be set here. The | ||
| # 4-GPU case (1536/4=384) shards cleanly and is left on the TP path. | ||
| if ('pytorch' in model_label and 'minimax_m2.5_fp8' in model_label.lower() | ||
| and 'gpus:8' in model_label.lower()): |
There was a problem hiding this comment.
Imprecise substring match for GPU count.
The substring check 'gpus:8' in model_label.lower() will match unintended labels like "gpus:80", "gpus:88", or any string containing "gpus:8" as a substring. This could incorrectly apply the 8-GPU MiniMax-M2.5 FP8 configuration to other GPU counts.
🔧 Proposed fix using more precise pattern matching
- if ('pytorch' in model_label and 'minimax_m2.5_fp8' in model_label.lower()
- and 'gpus:8' in model_label.lower()):
+ if ('pytorch' in model_label and 'minimax_m2.5_fp8' in model_label.lower()
+ and '-gpus:8-' in f'-{model_label.lower()}-'):
base_config['moe_expert_parallel_size'] = 8
base_config['enable_attention_dp'] = TrueThis wraps the label with delimiters to ensure "gpus:8" is an exact segment match, preventing false positives with "gpus:80" or "gpus:88".
🤖 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/integration/defs/perf/pytorch_model_config.py` at line 603, The current
substring check using model_label.lower() and 'gpus:8' can match unintended
labels like "gpus:80"; update the condition that decides the 8-GPU config to
perform an exact segment match instead of a raw substring—e.g., normalize
model_label, then either split the label on delimiters (spaces, commas, slashes,
colons) and compare tokens for the exact token "gpus:8", or use a regex with
anchors/boundaries to match r'(?<!\d)gpus:8(?!\d)'; change the conditional that
references model_label to use this precise match so only true 8-GPU labels
select the MiniMax-M2.5 FP8 configuration.
Summary
moe_expert_parallel_size=8andenable_attention_dp=True; removed the dead in-listep:8pattern block. Verified 2 passed, EXIT_CODE=0.Test plan
Links
Summary by CodeRabbit
Chores