-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[None][test] Enable overlap scheduler in the remaining perf sanity configs #16991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1931,8 +1931,9 @@ def _parse_disagg_config_file(self, config_file_path: str, config_file: str): | |
| ctx_config = dict(worker_config.get("ctx", {})) | ||
| # Ignore cache_transceiver_config for ctx_only | ||
| ctx_config.pop("cache_transceiver_config", None) | ||
| # Disable overlap scheduler for ctx_only | ||
| ctx_config["disable_overlap_scheduler"] = True | ||
| # The overlap scheduler setting is taken from the ctx worker config | ||
| # so that ctx_only measures the same scheduler path the ctx worker | ||
| # uses in e2e. ServerConfig defaults it to False when unset. | ||
|
Comment on lines
+1934
to
+1936
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' "Relevant parser and test references:"
rg -n -C 4 \
'ctx_only|disable_overlap_scheduler|PerfSanityTestConfig' \
tests/integration --glob '*.py' --glob '*.yaml' || true
printf '%s\n' "Relevant test-list references:"
rg -n -C 2 \
'perf_sanity|ctx_only|disable_overlap_scheduler' \
tests/integration/test_lists || trueRepository: NVIDIA/TensorRT-LLM Length of output: 50376 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' 'Changed files:'
git diff --name-only
printf '%s\n' 'Parser implementation:'
sed -n '1860,2010p' tests/integration/defs/perf/test_perf_sanity.py
printf '%s\n' 'Perf sanity test functions and parser assertions:'
rg -n -C 3 \
'^(def|async def) test_|parse_config_file|server_configs\[0\]|disable_overlap_scheduler' \
tests/integration/defs/perf tests/integration/test_lists \
--glob 'test_perf_sanity.py' --glob '*.py' --glob '*.yaml' \
| head -n 400
printf '%s\n' 'Perf sanity test-list files:'
git ls-files tests/integration/test_lists | rg -i 'perf|sanity|test-db|qa' | head -n 200Repository: NVIDIA/TensorRT-LLM Length of output: 29319 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' 'Perf sanity test-list entries:'
rg -n -i -C 3 \
'test_perf_sanity|perf_sanity|ctx_only' \
tests/integration/test_lists/qa/llm_perf_core.yml \
tests/integration/test_lists/qa/llm_perf_disagg.yml \
tests/integration/test_lists/test-db/l0_b200_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_b200_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_gb200_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_gb300_multi_gpus_perf_sanity.yml \
tests/integration/test_lists/test-db/l0_perf.yml \
2>/dev/null || true
printf '%s\n' 'All standalone tests targeting PerfSanityTestConfig:'
rg -n -i -C 4 \
'PerfSanityTestConfig|_parse_disagg_config_file|ctx_only' \
tests --glob '*.py' --glob '!tests/integration/defs/perf/test_perf_sanity.py' \
| head -n 300
printf '%s\n' 'ServerConfig initialization and scheduler field:'
sed -n '450,510p' tests/integration/defs/perf/test_perf_sanity.pyRepository: NVIDIA/TensorRT-LLM Length of output: 37701 🏁 Script executed: #!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/integration/defs/perf/test_perf_sanity.py")
source = path.read_text(encoding="utf-8")
tree = ast.parse(source)
server_default = None
ctx_only_branch = None
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
target_names = [
target.id for target in node.targets if isinstance(target, ast.Name)
]
if "self.disable_overlap_scheduler" in ast.unparse(node):
server_default = ast.unparse(node)
if isinstance(node, ast.If) and "benchmark_mode == \"ctx_only\"" in ast.unparse(node):
text = ast.unparse(node)
if "ctx_config = dict(worker_config.get(\"ctx\", {}))" in text:
ctx_only_branch = text
assert server_default == (
'self.disable_overlap_scheduler = '
'server_config_data.get("disable_overlap_scheduler", False)'
)
assert ctx_only_branch is not None
assert '**ctx_config' in ctx_only_branch
def parsed_value(ctx_config):
merged = {
"concurrency": -1,
"model_name": "model",
"disagg_run_type": "aggr",
**dict(ctx_config),
}
return merged.get("disable_overlap_scheduler", False)
cases = [
({"disable_overlap_scheduler": True}, True),
({"disable_overlap_scheduler": False}, False),
({}, False),
]
for config, expected in cases:
actual = parsed_value(config)
assert actual is expected, (config, actual, expected)
print("ctx_only merge preserves explicit True and False and defaults omitted key to False")
print("No focused parser test function is present in the repository search results")
PYRepository: NVIDIA/TensorRT-LLM Length of output: 246 🏁 Script executed: #!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/integration/defs/perf/test_perf_sanity.py")
tree = ast.parse(path.read_text(encoding="utf-8"))
print("Matching scheduler assignments:")
for node in ast.walk(tree):
if isinstance(node, ast.Assign) and "disable_overlap_scheduler" in ast.unparse(node):
print(ast.unparse(node))
print("ctx_only conditionals:")
for node in ast.walk(tree):
if isinstance(node, ast.If) and "ctx_only" in ast.unparse(node):
text = ast.unparse(node)
print(text[:1200].replace("\n", " "))
print("Relevant source lines:")
for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1):
if 1928 <= number <= 1953 or number == 494:
print(f"{number}: {line}")
PYRepository: NVIDIA/TensorRT-LLM Length of output: 7540 Add focused The parser preserves explicit Test coverage verdict: insufficient. 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| # Create server config for ctx_only (single ServerConfig, not tuple) | ||
| ctx_server_config_data = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment now explains code that isn't here, which reads oddly for someone landing on this block later. Consider folding it into the
ctx_configline above, e.g.# ctx_config carries disable_overlap_scheduler through as-is (ServerConfig defaults it to False) so ctx_only exercises the same scheduler path as the e2e ctx worker.