[PowerX] scope required power to selected 8k1k scenarios / 按所选 8k1k 场景要求功耗 - #3050
Conversation
为固定序列场景增加默认关闭的功耗要求,并贯通矩阵及工作流;保持 AgentX 和评估隔离。
|
Claude finished @edwingao28's task in 3m 9s —— View job Review of PR #3050
LGTM - no blocking issues found. Verified: the flag is emitted only from One non-blocking observation: the 8192/1024 restriction is enforced in |
| isl, osl = sequence[Fields.ISL.value], sequence[Fields.OSL.value] | ||
| require_power = sequence.get(Fields.REQUIRE_POWER.value, False) | ||
| if require_power and (isl, osl) != (8192, 1024): | ||
| raise ValueError("require-power rollout supports only fixed-sequence 8192/1024") |
There was a problem hiding this comment.
📊 Line Count Report
- Total Lines: 1594
- Base Lines: 1587
- Change: 📈 +7 lines
|
|
||
| isl: int | ||
| osl: int | ||
| require_power: bool = Field(default=False, alias=Fields.REQUIRE_POWER.value, strict=True) |
There was a problem hiding this comment.
🟡 (optional) SingleNodeSeqLenConfig/MultiNodeSeqLenConfig set populate_by_name=True, so a master config authored with require_power: true (snake_case) passes schema validation silently, but infx/matrix/generate.py:812 reads the raw yaml dict via sequence.get('require-power', False) (hyphen only) since validate_master_config returns the original dict, not the pydantic-normalized one. A misspelled key silently disables the power-validation requirement with no error anywhere. Fix: reject the non-alias spelling for this field (or normalize the validated model back into the dict actually consumed by generation) so config validation and the generator agree on one spelling for require-power and its multinode sibling at line 646.
Extended reasoning...
A config author writes require-power style keys everywhere else (matches Fields enum's kebab-case convention) but types require_power: true for this new field, following Python naming instead. SingleNodeSeqLenConfig(**entry) accepts it via populate_by_name and validate_master_config (infx/matrix/validation.py:957) returns the original master_configs dict unchanged (not the validated model). generate.py's _fixed_sequence_entries then does sequence.get(Fields.REQUIRE_POWER.value, False) i.e. sequence.get('require-power', False), which misses the snake_case key and returns False. The generated matrix entry has no require-power, so no ValueError, no REQUIRE_POWER env var, and CI never enforces the measured-power qualification the author believed they had enabled -- a silent no-op for a safety/qualification gate, with no diagnostic anywhere in the pipeline.
Verification: nit. The mechanism is real and reachable. validation.py:631 and :642 set ConfigDict(extra='forbid', populate_by_name=True) on SingleNodeSeqLenConfig/MultiNodeSeqLenConfig, and the new field is require_power: bool = Field(default=False, alias='require-power', strict=True) (lines 635, 646). With populate_by_name=True, pydantic accepts the snake_case field name require_power as valid input;…
矩阵生成同时识别校验允许的字段名与别名,避免静默丢失功耗门槛。
Description
Add an opt-in measured-power requirement to individual fixed 8192/1024 scenarios and propagate it through standard sweeps and E2E jobs. Existing recipes, AgentX and eval-only behavior remain unchanged.
Testing: Matrix CI passed at
671b0c1e5; two alias regressions failed before the fix, all four focused cases pass. No GPU run or recipe activation; runtime qualification and CODEOWNER approval remain required.中文
为独立固定 8192/1024 场景增加可选的实测功耗要求,并贯通标准 sweep 和 E2E 作业。现有配方、AgentX 和 eval-only 行为保持不变。
测试:
671b0c1e5的矩阵 CI 通过;两个字段别名回归用例在修复前失败,修复后四个针对性用例全部通过。未运行 GPU 或启用配方;仍需各 runtime 资格验证与 CODEOWNER 审批。Related Issue
Shared prerequisite for #3027 runtime splits.
#3027 runtime 拆分的共享前置。
Type of Change
Checklist
perf-changelog.yamland have not edited historical entriesOWNER/MEMBER/COLLABORATOR) has commented/reuse-sweep-runon this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.Note
Low Risk
CI matrix and generator changes only; eval-only and non-8k1k paths are explicitly excluded, with no auth or data-handling impact.
Overview
Adds an opt-in
require-powerflag on fixed-sequence master YAML (validated and emitted only for 8192/1024 rows) so benchmark matrix jobs can request validated measured power without changing AgentX or default behavior.The generator copies the flag onto throughput matrix rows, strips it from eval-only paths (
smoke, subset/all eval modes), and rejectsrequire-poweron other sequence lengths. Pydantic schemas accept the field on sequence configs and matrix entries.CI wiring:
run-sweepand manual E2E throughput jobs passrequire-powerwhen the matrix row sets it and the job is not eval-only; E2E also honors the workflow input or per-row flag. Docs (EN/ZH) describe rollout constraints. Tests cover single/multi-node and both key spellings.Reviewed by Cursor Bugbot for commit 671b0c1. Bugbot is set up for automated code reviews on this repo. Configure here.