[https://nvbugs/6187185][fix] Apply the existing low_memory_overrides() helper in `TestNemotronV2.test_auto_ - #14584
Conversation
…st_auto_dtype on <80GB GPUs The YAML config (nemotron-nano-9b-v2.yaml) sets max_batch_size=128 and free_gpu_memory_fraction=0.7 — values tuned for H100/B200 80GB+. On L40S 48GB, the Mamba state cache scales O(max_batch_size) and consumes ~8.8GB, leaving negative budget for the attention KV cache and tripping the 'Impossible to fit in any sequence in kvCache' assertion in get_max_atten_window_upper_bound(). Mirror the precedent in TestNemotronNanoV3.test_accuracy: when get_device_memory() < 80000 MiB, apply low_memory_overrides() to lower max_batch_size, free_gpu_memory_fraction, and align cuda_graph batch sizes. Preserve max_num_tokens (= 512 when chunked prefill is on). Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
📝 WalkthroughWalkthroughThe test ChangesNemotron V2 Memory Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
🧹 Nitpick comments (1)
tests/integration/defs/accuracy/test_llm_api_autodeploy.py (1)
503-503: QA list update is not needed for this change.This modifies runtime config behavior only; test node id/name/paramization did not change, so
tests/integration/test_lists/qa/llm_function_core.txtupdates are unnecessary.As per coding guidelines, “If a PR changes a test function/class name or its parameterization, update this node id list accordingly…”.
🤖 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/accuracy/test_llm_api_autodeploy.py` at line 503, The change only alters runtime config and did not rename or re-parameterize the test function test_auto_dtype (with its fixture enable_chunked_prefill), so revert any edits made to the QA node id list file tests/integration/test_lists/qa/llm_function_core.txt; ensure the node id list remains unchanged and only runtime-related test logic in test_auto_dtype is kept, without touching the QA list entries.
🤖 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.
Nitpick comments:
In `@tests/integration/defs/accuracy/test_llm_api_autodeploy.py`:
- Line 503: The change only alters runtime config and did not rename or
re-parameterize the test function test_auto_dtype (with its fixture
enable_chunked_prefill), so revert any edits made to the QA node id list file
tests/integration/test_lists/qa/llm_function_core.txt; ensure the node id list
remains unchanged and only runtime-related test logic in test_auto_dtype is
kept, without touching the QA list entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8e46dd9e-6d0b-4e48-a3c8-2221fdfc3aac
📒 Files selected for processing (1)
tests/integration/defs/accuracy/test_llm_api_autodeploy.py
|
/bot run |
|
PR_Github #50364 [ run ] triggered by Bot. Commit: |
|
PR_Github #50364 [ run ] completed with state
|
jieli-matrix
left a comment
There was a problem hiding this comment.
plz use decorator in conftest instead of adding if-else branch inside the test function; skip test decorator can help to skip the test case before running it, which means better execution efficiency & clear code structure.
Signed-off-by: Chenghao Zhang <211069071+nvchenghaoz@users.noreply.github.com>
|
@jieli-matrix thanks for the review! Updated the PR. Could you please take a look? |
|
/bot run |
|
PR_Github #50816 [ run ] triggered by Bot. Commit: |
|
PR_Github #50816 [ run ] completed with state
|
|
/bot run |
|
PR_Github #50822 [ run ] triggered by Bot. Commit: |
|
PR_Github #50822 [ run ] completed with state
|
|
/bot run |
|
PR_Github #50846 [ run ] triggered by Bot. Commit: |
|
PR_Github #50846 [ run ] completed with state |
Summary
max_batch_size=128(Mamba state O(max_batch_size) ≈ 8.8GB) + 16.5GB BF16 weights + forward-pass peak leaves negative budget for the attention KV cache.low_memory_overrides()helper inTestNemotronV2.test_auto_dtypewhenget_device_memory() < 80000MiB, mirroring the precedent inTestNemotronNanoV3.test_accuracy; preservemax_num_tokensso the chunked-prefill variant keeps its 512-token override.Test plan
Links
Summary by CodeRabbit