Clean up configuration ownership and redundant defaults after PR #2127 / 清理 PR #2127 后的配置归属与冗余默认值

Open
#2,150 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
38/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Quiet
Tech stack
bash, python, yaml

Research direction

Rebase onto main after #2127, then audit benchmarks/multi_node/agentic/dsv4_fp4_mi355x_sglang-disagg.sh and the listed amd_utils and launcher files using the ownership rules. Review .github/workflows/run-sweep.yml and utils/matrix_logic/generate_sweep_configs.py separately, then run bash -n, YAML parsing, matrix and Agentic aggregation tests, and dry-run checks. Done means redundant defaults and self-exports are removed, scope is separated, and the DSV4 matrix and workflow inputs remain correct.

Written by the indexing model from the issue text.

Description

Summary

Follow up after #2127 merges to clean up configuration ownership and redundant shell defaults across the multi-node DSV4 Agentic path.

Do not modify #2127 for this cleanup; preserve its validated sweep and /reuse-sweep-run merge path. Start the implementation from main after #2127 is merged.

Cleanup rules

Apply these rules consistently across every file introduced or modified by #2127:

  • Caller/workflow-owned inputs must be required and consumed directly. Do not add ${VAR:-default} after the caller has already supplied or validated the variable.
  • Recipe-owned policy must be assigned once as an explicit value in the recipe.
  • Retain an environment override only when it is an intentional, documented interface with a real caller or operational use.
  • Remove pass-through self-assignments such as export MODEL_PATH=$MODEL_PATH when they do not change scope or behavior.
  • Restore unrelated global behavior to main or move it to a separate infrastructure PR.
  • Keep the DSV4 recipe specific to dsv4-fp4-mi355x-sglang-disagg-agentic-hicache.

Initial findings

benchmarks/multi_node/agentic/dsv4_fp4_mi355x_sglang-disagg.sh

Audit every assignment. In particular:

  • DURATION and KV_OFFLOADING are already checked by check_env_vars; consume them directly without defaults.
  • MODEL_PREFIX, PRECISION, RESULT_FILENAME, MAX_MODEL_LEN, topology, concurrency, and framework values are supplied by the reusable workflow or launcher; do not silently replace them.
  • Decide explicitly which of TIME_LIMIT, HiCache settings, MoRI queue tuning, router policy, metrics, and MTP settings are recipe-owned constants versus real supported caller inputs.
  • Remove redundant self-exports and fallback chains.
  • Derive EP/DP flags from already-required topology inputs without adding fallback values.
Shared runtime files

Review these against the same ownership model:

  • benchmarks/multi_node/amd_utils/env.sh
  • benchmarks/multi_node/amd_utils/job.slurm
  • benchmarks/multi_node/amd_utils/models.yaml
  • benchmarks/multi_node/amd_utils/server_sglang.sh
  • benchmarks/multi_node/amd_utils/setup_deps.sh
  • benchmarks/multi_node/amd_utils/submit.sh
  • benchmarks/multi_node/amd_utils/trace_replay.sh
  • runners/launch_mi355x-amds.sh

For each added default or override, identify its owner and caller before retaining it.

Generic infrastructure scope

Reassess generic changes separately from recipe cleanup:

  • .github/workflows/run-sweep.yml array serialization for multi-node Agentic conc-list/conc.
  • utils/matrix_logic/generate_sweep_configs.py changing from up to four concurrencies per allocation to one allocation per concurrency.

The workflow serialization fix may be valid generic infrastructure, but should not be hidden inside a recipe cleanup. The global one-concurrency scheduling policy requires an explicit independent decision.

Validation

After cleanup:

  • rebase onto post-#2127 main;
  • run bash -n on every changed shell/Slurm script;
  • parse all changed YAML;
  • run matrix logic tests;
  • run Agentic aggregation tests;
  • run relevant dry-run/server command generation checks;
  • verify the generated DSV4 matrix and workflow inputs;
  • confirm no unrelated existing config changes;
  • use a new sweep only if runtime behavior changes.

中文说明

在 #2127 合并后,针对多节点 DSV4 Agentic 路径进行一次独立的配置归属和冗余默认值清理。不要为了本次清理继续修改 #2127,以免影响已经验证通过的扫描结果和 /reuse-sweep-run 合并流程;实现应从 #2127 合并后的 main 开始。

统一遵循以下规则:

  • 由 workflow 或调用方提供的输入必须直接使用;如果变量已经由调用方设置或通过校验,不要再次使用 ${VAR:-default}
  • 属于配方自身策略的参数应在配方中明确赋值一次。
  • 只有确实存在调用方或运维场景、并作为正式接口记录的参数,才保留环境变量覆盖能力。
  • 删除不改变作用域或行为的自赋值,例如 export MODEL_PATH=$MODEL_PATH
  • 与配方无关的全局行为应恢复到 main,或拆分为独立的基础设施 PR。
  • DSV4 配方应仅服务于 dsv4-fp4-mi355x-sglang-disagg-agentic-hicache

需要逐文件审查 #2127 涉及的 recipe、共享 runtime、launcher、workflow 和 matrix 逻辑,明确每个默认值和覆盖项的所有者及真实调用链。清理完成后,执行 Shell、YAML、Python、矩阵逻辑、Agentic 聚合以及相关 dry-run 验证,并确认没有修改无关的既有配置。

Dominant language
Python
Stars
1.7k
Forks
303
Avg merge
1d 13h
Merged PRs (30d)
284

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from SemiAnalysisAI/InferenceX

All issues in SemiAnalysisAI/InferenceX

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.