Skip to content

[nv] dsv4 b200 agentic points at conc 14 and 18 - #2702

Open
xinli-sw wants to merge 1 commit into
mainfrom
dsv4-b200-append-points
Open

[nv] dsv4 b200 agentic points at conc 14 and 18#2702
xinli-sw wants to merge 1 commit into
mainfrom
dsv4-b200-append-points

Conversation

@xinli-sw

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from ad26169 to 544a1b8 Compare August 21, 2026 02:11
@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from 544a1b8 to 557fb30 Compare August 21, 2026 02:14
@github-actions

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional findings (outside current diff — PR may have been updated during review):

  • 🟡 utils/matrix_logic/validation.py:615-627 — AgenticCodingSearchSpaceEntry.validate_topology_fields() (utils/matrix_logic/validation.py:615-627, forbid check ~line 656) forbids pp/dcp-size/pcp-size on multinode (prefill+decode) agentic search-space entries, but the new max-num-seqs/long-prefill-token-threshold fields added here were not added to that forbidden set. These two fields are single-node-only in practice (only read/wired by the single-node branches of generate_full_sweep/generate_test_config_sweep, and sweep-multi-node-agentic never passes them; MultiNodeAgenticMatrixEntry uses extra=forbid with no such fields), so a config author who mistakenly sets either field on a multinode agentic entry passes validation silently and the value is just dropped, instead of failing loudly the way an accidental pp/dcp-size/pcp-size does.

    Extended reasoning...

    What the bug is: validate_topology_fields() in AgenticCodingSearchSpaceEntry has an explicitly_single_node_fields set that forbids pp, dcp_size, and pcp_size from being set on a search-space entry that also has complete multinode fields (prefill and decode both present). This PR adds two new optional fields to the same model, max_num_seqs and long_prefill_token_threshold (validation.py:618-624), but does not add them to explicitly_single_node_fields (validation.py, the forbid check around line 656).

    Why they are single-node-only in practice: In generate_sweep_configs.py, both generate_full_sweep and generate_test_config_sweep only read bmk.get(Fields.MAX_NUM_SEQS.value) / bmk.get(Fields.LONG_PREFILL_TOKEN_THRESHOLD.value) inside the single-node else branch of the is_multinode conditional, and only wire them into the emitted entry inside the single-node per-concurrency loop. The multinode branch of both generators never reads or emits these fields. Downstream, sweep-agentic/sweep-agentic-evals in run-sweep.yml (single-node dispatch) pass max-num-seqs/long-prefill-token-threshold through to benchmark-tmpl.yml, but sweep-multi-node-agentic never does. MultiNodeAgenticMatrixEntry also has model_config = ConfigDict(extra='forbid') and declares no such fields, so even if a value did reach that model it would hard-fail there — but it never gets that far, since the generator drops it first.

    The resulting inconsistency: if a master-config author sets pp: 2 (or dcp-size/pcp-size) by mistake on a multinode agentic search-space entry, validate_topology_fields() immediately raises a clear validation error at config-load time. But if the same author mistakenly sets max-num-seqs or long-prefill-token-threshold on that same multinode entry, validation passes silently, and the value is quietly dropped by the generator with no error or warning anywhere in the pipeline — the multinode agentic job simply runs without ever seeing the intended override.

    Step-by-step proof:

    1. Author adds a multinode agentic search-space entry with prefill: {...}, decode: {...}, max-num-seqs: 32 to a master config.
    2. load_config_files -> validate_master_config -> AgenticCodingSearchSpaceEntry(**entry) parses successfully: max_num_seqs=32 is accepted by the field definition, and validate_topology_fields only checks {pp, dcp_size, pcp_size} against model_fields_set, which does not include max_num_seqs, so no error is raised.
    3. generate_full_sweep/generate_test_config_sweep reach the is_multinode branch for this entry; max_num_seqs is simply never read (it's only read in the else branch), so it is not placed into the constructed entry dict.
    4. The generated multinode matrix entry validates against MultiNodeAgenticMatrixEntry (extra=forbid) successfully, since the field was never added.
    5. sweep-multi-node-agentic in run-sweep.yml dispatches the job with no max-num-seqs input at all — the author's override is gone with zero errors or warnings anywhere in the chain.

    Impact and fix: Nothing crashes and the currently added recipe in this PR (single-node TP8 dsv4 points) is unaffected — it exercises only the single-node path, where these fields are handled correctly end-to-end. This is purely a defensive-validation gap: a future author extending a multinode agentic entry with these fields would hit silent, hard-to-diagnose config loss rather than an immediate, actionable error. The fix is small: add "max_num_seqs" and "long_prefill_token_threshold" to the explicitly_single_node_fields set in validate_topology_fields(), mirroring how pp/dcp_size/pcp_size are already handled.

@github-actions

Copy link
Copy Markdown
Contributor

@xinli-sw xinli-sw changed the title [nv] dsv4 b200 agentic points at conc 18 and 20 [nv] dsv4 b200 agentic points at conc 14 & 18 Aug 21, 2026
@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from 557fb30 to f308e40 Compare August 21, 2026 03:51
@xinli-sw xinli-sw changed the title [nv] dsv4 b200 agentic points at conc 14 & 18 [nv] dsv4 b200 agentic points at conc 14 and 18 Aug 21, 2026
@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from 04626c9 to 9e5dc0e Compare August 21, 2026 03:53
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from 9e5dc0e to 845436a Compare August 21, 2026 05:34
@github-actions

Copy link
Copy Markdown
Contributor

@xinli-sw
xinli-sw force-pushed the dsv4-b200-append-points branch from 845436a to e401ba1 Compare August 21, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant