[https://nvbugs/6581048][test] Unwaive TorchSampler beam-search e2e case - #17830
Conversation
Retested on main at bac9a5c (1.3.0rc25), built for sm80 and run on an A30 -- the same GPU l0_a30.yml runs this case on. The target case passed 21/21 consecutive runs and the whole test_beam_search.py passed 345/0. The failure is no longer reproducible, so remove the waiver. Signed-off-by: ZhaoyangWang <zhaoyangw@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review. WalkthroughThe change removes a skip entry for the multi-process ChangesTorchSampler test execution
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change restores one previously waived beam-search test to pre-merge coverage after repeated successful runs; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run |
|
PR_Github #66766 [ ] completed with state |
|
/bot run |
|
PR_Github #66774 [ run ] triggered by Bot. Commit: |
|
PR_Github #66774 [ run ] completed with state
|
|
/bot run |
|
PR_Github #66904 [ run ] triggered by Bot. Commit: |
|
PR_Github #66904 [ run ] completed with state |
Description
test_beam_search_e2e[multi_process-TorchSampler-no_cuda_graph_and_overlap-stop_token_ids0-1-1-True-True-True]was waived under https://nvbugs/6581048. The failure is no longer reproducible on
current main, so this PR removes the waiver and puts the case back into pre-merge.
Verification was done on an A30 -- the same GPU
l0_a30.ymlschedules this file on --with main built locally at
bac9a5cc58(1.3.0rc25,-a 80-real):tests/unittest/_torch/sampler/test_beam_search.pyEach of the 20 iterations ran in its own pytest process, so the
LLMobject and theMPI worker pool were rebuilt every time rather than reused.
Because this parametrization runs with
multi_process, MPI worker spawning was checkedbefore the test (
MPI_Initplus anMPIPoolExecutorspawn round-trip) to make sure adegraded single-process fallback could not turn into a false pass.
The 244 skips are the test's own
pytest.skipguards for TRTLLMSampler combinations(
return_log_probswith multiple prompts, repeatedlog_probscalls,stop_token_ids),not environment-related.
Test Coverage
unittest/_torch/sampler/test_beam_search.py::test_beam_search_e2e[multi_process-TorchSampler-no_cuda_graph_and_overlap-stop_token_ids0-1-1-True-True-True]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)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.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.
Dev Engineer Review
TorchSamplerbeam-search waiver fromtests/integration/test_lists/waives.txt.QA Engineer Review
test-db/orqa/files were modified.TorchSamplerbeam-search end-to-end test.