Repository navigation
feat(tests): add tool-call concurrency benchmark harness (Issue #10) and spark_doctor diagnostics - #41
Conversation
…-Lab#10) and spark_doctor diagnostic script
|
Thank you, investigating. |
Live retest of this harness on current
|
| Wave | N | Timeouts | Blank {} / missing required args |
Finish |
|---|---|---|---|---|
| c=1, no pad | 1 | 0 | 0 | tool_calls |
| c=4, no pad | 4 | 0 | 0 | tool_calls |
c=4, --filler-words 15000 (~15k prompt tokens, #10 ~60k-char pad) |
4 | 0 | 0 | tool_calls |
c=8, --filler-words 15000 (the synthetic shape that reported 1 blank after timeouts) |
8 | 0 | 0 | tool_calls |
| Mixed: 1×15k cold peer + 3 short | 4 | 0 | 0 | tool_calls |
c=4, --thinking |
4 | 0 | 0 | tool_calls |
c=4, --filler-words 48000 (~48k prompt tokens, as advertised in this PR) |
4 | 0 | 0 | tool_calls |
| Total | 29 | 0 | 0 |
Every success filled both calls, e.g. terminal_execute({"command":"echo hello-concurrency-test"}) and browser_action({"action":"open","url":"https://example.com/test-issue10"}). Decode stayed ~30–54 tok/s. /health 200 after the 48k-token wave (TTFT up to ~200 s; skip serializes the long prefills).
So on this kit the harness works as a detector, but it does not reproduce #10. That matches the earlier 0/53 synthetic on this recipe.
@Synxneuos can you re-run this same harness on latest main (not the original seqs=8 / MNBT=1024 boot) and post the receipt? If it still blanks for you, we need the exact ./start.sh knobs, max_num_seqs / max_num_batched_tokens, and whether any lane was finish_reason=length vs tool_calls with {}.
|
Thanks @MiaAI-Lab for testing it on the live 2× GB10 cluster and sharing the receipt! Confirmed — on latest The harness was designed as a push-button detector to verify that If you'd like, we can also contribute the LENGTH-truncation guard overlay patch (preventing |
plotarmordev
added
triage/P2
Review follow-up —
|
…e tests tests/test_start_overrides.py was rewritten against a stale base: the exact stdout assert became a substring check, the isolated env became os.environ.copy() plus a synthetic .env.example, and an os.name == "nt" early return asserted per-knob _cli_* source text that main no longer has. None of that belongs to this PR's concurrency-detector scope, and main has since moved every caller knob to the generic _caller_overrides snapshot with setness-aware _run_preamble coverage. Take main's version verbatim (sha256 3b5ca8ded1f4): exact equality, the isolated env, and the setness/precedence tests are restored. The concurrency detector, spark_doctor.sh and the encoding hygiene hunks are untouched.
…anup-7c8b Brings the PR's stale base (79f10b9) up to current main (b3bd1f6) on the feature branch so the concurrency detector, spark_doctor and encoding hygiene hunks sit on the reviewed head. tests/test_start_overrides.py is main's version on both sides after the preceding commit, so the caller precedence contract is unchanged by this merge; main's other changes to the branch-touched files are taken as-is.
scripts/ is invoked as ./scripts/<name>.sh — boot-shape-warmup.sh, boot_candidate.sh and boot_live_best.sh are all 100755 — but spark_doctor.sh was committed 100644, so ./scripts/spark_doctor.sh died with "Permission denied" while `bash scripts/spark_doctor.sh` worked. One-bit mode fix, no content change.
The band check piped through `bc -l 2>/dev/null || echo 0`: on a host without bc (or with a non-numeric value) the substitution yielded 0, both comparisons fell through, and any value printed the in-band PASS line. The value is now judged with start.sh's validate_numeric_config accepted set (same regex plus awk 0 < util <= 1); anything unjudgeable is a FAIL, says the UMA budget cannot be judged, and leaves the run at exit 1 instead of a silent PASS. Also corrected in the same review pass: - default GPU_MEM_UTIL now tracks start.sh:155 / .env.example:176 (0.85, not the retired 0.87) - the PASS line states the interval actually checked (0.70 - 0.95) and claims band sanity only, not a "recommended recipe envelope (0.80 - 0.90)" or safe memory admission - the header listed weights-cache integrity, GID table, round-trip latency, container toolkit and token-generation latency checks that the script never performed; it now describes the five checks that exist and says a clean run is not a serving-readiness verdict - /health 200 and the summary are reported as endpoint reachability / preflight scope instead of "Service Active & Healthy" and "ready for ... serving" tests/test_spark_doctor.sh adds inert CPU coverage: every probe the doctor makes (ping, ssh, ip, ibv_devinfo, nvidia-smi, docker, curl) is stubbed on PATH and the doctor runs from a throwaway copy of scripts/, so no node, Docker daemon, GPU or network is touched. It pins the FAIL/PASS/WARN verdicts above and the executable-mode convention.
…overage main() formatted ttft_s/tok_s with .2f/.1f on values that stay None when a successful stream carries no usage block, so the lane print raised TypeError before the JSON receipt was written. Absent measurements now print as "unavailable" and the receipt keeps None instead of a substituted 0. The module header claimed a non-streaming comparison and required-argument validation that the detector never performed; it now names the blank/malformed check it actually runs and states both absences. Drop the incidental padded-prompt and tool-schema tests, and add the missing-usage receipt regression.
…rdev/pr41-cleanup-7c8b
plotarmordev
left a comment
plotarmordev
left a comment
There was a problem hiding this comment.
SIX exact-head approval: 967fe4c. Reviewed all carried detector/doctor code and narrow UTF-8/newline hygiene changes. Current-main tests/test_start_overrides.py is preserved verbatim; focused launcher and concurrency tests passed (9), and all inert doctor probes passed on this integrated head. Missing SSE usage now retains null timing and writes its receipt without a formatting crash; Main separately demonstrated the regression failing the old head. Required-argument validation/non-stream comparison are explicitly not claimed; no parser/truncation fix is included. Publishing tree and entire pushed range scanned; source branch/authorship preserved. No workflows/checks exist, not CI green. No live doctor, nodes, model, GPU, SSH, container or service operations; doctor is preflight only and concurrency is detection only.
…rdev/pr41-cleanup-7c8b
plotarmordev
left a comment
plotarmordev
left a comment
There was a problem hiding this comment.
SIX renewed exact-head approval: 70fb7c6. Normally integrated externally advanced main b517eef (PR187) without altering its changes. Re-ran the final-head inert doctor probes and focused launcher/concurrency suite: all probes and 9 tests passed. Original code review and resolved missing-usage crash remain applicable; launcher precedence file preserved. Complete final publishing tree and full original-to-final pushed history rescanned: no secrets or owner-local deployment modifications. Historical author PR70 endpoint exists only in already-public immutable ancestry, removed from final tree. No workflows/checks exist; no live/GPU/SSH/container/service operations or new serving qualification.
plotarmordev
removed
the
merge-hold
Summary
This PR addresses the reproduction and validation requirements requested by @MiaAI-Lab in Issue #10 ("Tool calls sometimes emitted with blank/missing required arguments under concurrent load"), and adds a cluster preflight hardware diagnostic tool:
tests/test_tool_concurrency.py:-c 1, 4, 8).--filler-words N) to stress KV & prefix-caching interactions.{}/ missing required arguments or malformed JSON.finish_reason=tool_callsvsfinish_reason=lengthtruncation.--out /tmp/tool-concurrency-receipt.json) capturing TTFT, tok/s, prompt/completion tokens, cached tokens, and per-lane status.scripts/spark_doctor.sh:HEAD_CX7_IF,WORKER_CX7_IF, GID table, and link state).GPU_MEM_UTIL(0.80 - 0.90 envelope)./healthand OpenAI/v1/modelsendpoint verification.Cross-Platform Test Robustness:
encoding="utf-8"inread_text()calls across tests (test_bringup_robustness.py,test_start_overrides.py,test_warm_restart_stdout.py,test_xgrammar_termination.py) so test suites run cleanly across Linux and Windows developer environments.tests/test_tool_concurrency_unit.py(4/4 tests passing).Test Proof
pytest tests/test_tool_concurrency_unit.py -v(4 passed in 0.13s).pytest tests -v(21 passed in 1.81s).