[TRTLLM-13546][feat] Add error classification patterns (1c.1)#15677
Conversation
chienchunhung
changed the title
chienchunhung
changed the title
📝 WalkthroughWalkthroughError classification patterns are expanded for additional fatal and severe CUDA, NCCL, MPI, RDMA, and NIXL messages. A new test module covers the updated classifications and ChangesError classification coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unittest/_torch/pyexecutor/test_error_classification.py (1)
1-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRerun
ruff-formaton the changed Python files before merge.The current patch is blocked by pre-commit:
ruff-formatmodified one changed Python file, so CI will keep failing until the formatter output is committed.🤖 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/pyexecutor/test_error_classification.py` around lines 1 - 84, The patch needs formatting cleanup because ruff-format is changing one of the edited Python files, so the change set is not in its final committed form. Re-run the formatter on the affected test module, especially the import block and parameterized test definitions in test_error_classification.py, then commit the formatted result so the pre-commit ruff-format check passes.Source: Pipeline failures
🤖 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/pyexecutor/error_classification.py`:
- Line 39: The immediate-fatal token list in classify_error() is too broad
because the bare string "unrecoverable" can match unrelated errors and trigger
shutdown via ErrorBudget.consume() in py_executor.py. Replace that entry in the
immediate-fatal matching set with the exact emitted phrase you intend to catch,
using classify_error() and the fatal-error constants in error_classification.py
as the location to update. Keep the match specific enough to only classify the
intended WideEP failure path and avoid false positives from other libraries or
user code.
In `@tests/unittest/_torch/pyexecutor/test_error_classification.py`:
- Around line 35-60: Add direct regression coverage for the new compact literal
spellings in test_error_classification.py. The current parameterized cases in
test_wide_ep_immediate_fatal_patterns and test_wide_ep_severe_patterns cover
descriptive messages, but they do not verify the exact substrings
cudaerrorcontextisdestroyed and ncclcommabort. Add explicit message cases to
classify_error assertions so a typo in the literal lists would fail these tests.
---
Outside diff comments:
In `@tests/unittest/_torch/pyexecutor/test_error_classification.py`:
- Around line 1-84: The patch needs formatting cleanup because ruff-format is
changing one of the edited Python files, so the change set is not in its final
committed form. Re-run the formatter on the affected test module, especially the
import block and parameterized test definitions in test_error_classification.py,
then commit the formatted result so the pre-commit ruff-format check passes.
🪄 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: 83daf61e-54c7-469a-950f-5d22ff712026
📒 Files selected for processing (3)
tensorrt_llm/_torch/pyexecutor/error_classification.pytests/integration/test_lists/test-db/l0_a10.ymltests/unittest/_torch/pyexecutor/test_error_classification.py
chienchunhung
force-pushed
the
WideEP-FT/1c.1-error-classification
branch
2 times, most recently
from
4f6ca77 to
7b11e41
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/unittest/_torch/pyexecutor/test_error_classification.py`:
- Around line 36-68: Coverage in test_wide_ep_immediate_fatal_patterns and the
adjacent WideEP timeout parametrization is still missing exact-message
regressions for several new literals, so add direct cases in
test_error_classification.py for the new spellings seen in
error_classification.py. Cover the specific substrings like cuda context
destroyed, mpi worker exited unexpectedly, nccl timeout, completion_flags
timeout, and both deep_ep/deepep buffer barrier hang variants, and assert they
map to the expected classifications so typos in the pattern list are caught
immediately.
🪄 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: 78899fe7-e5b1-4b53-8e77-af275445cb42
📒 Files selected for processing (3)
tensorrt_llm/_torch/pyexecutor/error_classification.pytests/integration/test_lists/test-db/l0_a10.ymltests/unittest/_torch/pyexecutor/test_error_classification.py
chienchunhung
force-pushed
the
WideEP-FT/1c.1-error-classification
branch
from
7b11e41 to
331f9ee
Compare
|
/bot run --disable-fail-fast |
chienchunhung
changed the title
|
PR_Github #56159 [ run ] triggered by Bot. Commit: |
|
PR_Github #56159 [ run ] completed with state
|
chienchunhung
force-pushed
the
WideEP-FT/1c.1-error-classification
branch
from
331f9ee to
0036127
Compare
|
/bot run |
|
PR_Github #56269 [ run ] triggered by Bot. Commit: |
|
PR_Github #56269 [ run ] completed with state
|
chienchunhung
force-pushed
the
WideEP-FT/1c.1-error-classification
branch
from
0036127 to
31adca1
Compare
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung
force-pushed
the
WideEP-FT/1c.1-error-classification
branch
from
31adca1 to
d1da4bd
Compare
|
/bot run |
|
PR_Github #56433 [ run ] triggered by Bot. Commit: |
|
PR_Github #56433 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #56651 [ run ] triggered by Bot. Commit: |
|
PR_Github #56651 [ run ] completed with state
|
|
/bot run --disable-fail-fast --stage-list "DGX_H100-PyTorch-3" |
|
PR_Github #56710 [ run ] triggered by Bot. Commit: |
|
PR_Github #56710 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #56732 [ run ] triggered by Bot. Commit: |
|
PR_Github #56732 [ run ] completed with state |
3 participants
Summary
Design scope
This implements the 1c.1 pattern-only portion of the WideEP fault-tolerance roadmap. It does not add new severity classes, per-rank budgets, or routing/reconfiguration behavior.
Tests
python3 scripts/test_to_stage_mapping.py --tests unittest/_torch/pyexecutor/test_error_classification.py->A10-PyTorch-1,A10-PyTorch-2python3 -m py_compile tensorrt_llm/_torch/pyexecutor/error_classification.py tests/unittest/_torch/pyexecutor/test_error_classification.pytest_error_classification.pyassertions with a stub forpytest.mark.parametrizegit diff --checkpytestwas not available in the local environment (No module named pytest).Summary by CodeRabbit