Repository navigation
Use Magpie's native AgentX contract for optimization - #1922
Conversation
Code metrics gate: PASSEDJudged: the 130 files this change touches. Files it does not touch never fail it; anything found there is listed under Outside this change for information.
Module length warning: over 800 lines, within the 1200-line failure limit (not failing): 4
Already over at the base (backlog, not this change's): 32
...and 7 more; the job log lists every row. 61 findings in files this change does not touchOutside this change (informational): 61
...and 36 more; the job log lists every row.
Thresholds and sources: |
haofrank
added
the
size-exception
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| run = asyncio.create_task(coordinator.phase_kernel._revalidate_geak_candidate(reason="unit")) | ||
| await _actual_verdict(coordinator, "approve") | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await run |
There was a problem hiding this comment.
Keeping this: await run waits for the task and propagates asyncio.CancelledError into pytest.raises. It is the assertion point for cancellation, not an unused expression. The cancellation/resume regression passes.
| envs[str(k)] = str(v) | ||
| from ._workload_envs import pin_mlperf_round_concurrency, prepend_launch_path | ||
| base_envs = _apply_variant_env(envs, variant, base_extra_envs, base_unset_envs) | ||
| from ._workload_envs import pin_mlperf_round_concurrency |
There was a problem hiding this comment.
This function-local import already exists on main. The reported return path depends on the type-only Coordinator import in collaborator.py, so it does not form an executable import cycle. Keeping the shared concurrency helper in its current owner.
| log.error("grid_runner: %s; failing this benchmark", eval_bounds_msg) | ||
| return PreparationFailure(EVAL_PROBE_UNPATCHABLE_RETURNCODE, eval_bounds_msg) | ||
| log.warning("grid_runner: %s", eval_bounds_msg) | ||
| from ._workload_envs import prepare_agentx_runtime |
There was a problem hiding this comment.
This preparation step was moved from _run_magpie; its _workload_envs import remains deferred until the function runs. The return path crosses collaborator.py's TYPE_CHECKING import, which is not executed. No runtime cycle was introduced.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| run = asyncio.create_task(coordinator.phase_kernel._revalidate_geak_candidate(reason="unit")) | ||
| await _actual_verdict(coordinator, "approve") | ||
| with pytest.raises(asyncio.CancelledError): | ||
| await run |
There was a problem hiding this comment.
Keeping this: await run waits for the task and propagates asyncio.CancelledError into pytest.raises. It is the assertion point for cancellation, not an unused expression. The cancellation/resume regression passes.
jiaqiang-dot-liu
left a comment
jiaqiang-dot-liu
left a comment
There was a problem hiding this comment.
PR #1922 -- Use Magpie's native AgentX contract for optimization
What it does: Hyperloom carried its own AgentX execution stack -- a shell AIPerf client, a phase
gate, an installer/repair path and a corpus-rules mirror -- alongside Magpie, which already owns
server launch, replay, profiling and cleanup, so the two had to be kept in step by hand. This PR
deletes Hyperloom's copy and makes Magpie the sole owner: HYPERLOOM_AGENTX=1 now translates the
ordinary workload flags into a native Magpie YAML (or --benchmark-config supplies one), the
resolved workload is archived as <session>/benchmark.yaml and re-read on resume, candidates are
composed into benchmark.agentx.launch_overrides instead of into env and extra-args, and KEEP
evidence comes from Magpie's public benchmark_report.json validity verdict rather than from a
parsed AIPerf export. Profile rounds become diagnostic-only and can never justify a KEEP; reviewed
GEAK source products get a new revalidation phase that routes through Critic review and the existing
integrate_patch executor.
Blocking issues: 1
- [D11] Magpie's server-launch receipt is written under
effective_envand read back under
runtime_environment, so the GEAK handoff discards the accepted server environment [verified]
Problem:native_launch_evidencetakes the environment off Magpie'sserver_launchas
launch["effective_env"]and republishes that same dict verbatim asmagpie_server_launch
(src/hyperloom/inference_optimizer/agentx/report.py:80 and :107 -- and
test_magpie_agentx_contract.py:234 asserts the republication is verbatim through the real
BaselineExecutor).seed_native_geak_proxyreads that object back out and takes the
environment asevidence.get("runtime_environment")
(src/hyperloom/inference_optimizer/agentx/geak_proxy.py:82). It is one object: baseline.py:557
attaches it toresult["launch_evidence"], writeback.py:3419 shallow-copies the evidence
without touchingmagpie_server_launch, kernel.py:1297 reads it back off
current_best_measurement, and kernel.py:1483 hands it toseed_native_geak_proxy. The two
names cannot both be Magpie's. A tree-wide grep findsruntime_environmentused as a mapping
key in exactly one production line (geak_proxy.py:82) andeffective_envin exactly one
(report.py:80).
Impact: if Magpie's key iseffective_env,runtimeis PATH-only, and because geak_proxy.py:90
doesconfig.update({..., "extra_envs": runtime})-- an overwrite, not a merge -- the handoff
also discards the cumulative accepted environment it already carried from
baseline_env_spec.config(kernel.py:1468; writeback.py:3504 documents that field as "cumulative
server args + env vars").handoff["accepted_env"]likewise comes back empty, and both are
serialized togeak/handoff.json(kernel.py:1486-1488), so GEAK tunes kernels against a server
configuration missing every environment variable the accepted launch ran with, silently and on
every native session --seed_native_geak_proxyraises rather than returning when the receipt is
absent, so this is the live branch, not a fallback. If instead Magpie's key is
runtime_environment, report.py:80's bare subscript raisesKeyError: 'effective_env'on the
first native baseline that produces launch evidence. One of the two is live; neither is
acceptable. CI is green over it because the two new test files encode the two different names and
never cross: test_magpie_agentx_contract.py:133 authorseffective_envand drives the real
executor but never callsseed_native_geak_proxy, while
test_geak_handoff_workload_contract.py:135 hand-writes amagpie_server_launchcarrying
runtime_environmentand noeffective_env-- a receipt the production writer cannot produce,
and one that would have raisedKeyErrorhadnative_launch_evidencebuilt it (T1).
Action: read the key name off Magpie at the pinned6a1ee26f9053363d3750b43e3d5afb5d1c4ce84eand
use that one name in both places -- better, haveseed_native_geak_proxyconsume the
requested_server_envthatnative_launch_evidencealready publishes rather than re-deriving it
from the raw receipt -- merge rather than overwriteconfig["extra_envs"], and add one test that
carries a receipt fromnative_launch_evidencethrough tohandoff["accepted_env"]so the two
halves can no longer drift.
One limit, stated rather than hidden: Magpie is external and itsserver_launchschema is not
vendored or documented in this tree, so neither I nor the independent reviewer could rule out
that Magpie emits both keys, in which case this degrades to two spellings of one field. The
in-repo evidence points the other way -- each of the two new fixtures carries exactly one of the
keys and omits the other -- and the first action above settles it either way.
Checked: src/hyperloom/inference_optimizer/agentx/{report.py,geak_proxy.py,runtime.py,deploy.py,mapping.py},
src/hyperloom/inference_optimizer/cli/{benchmark_config.py,init.py,parser.py,bootstrap.py,preflight.py},
src/hyperloom/inference_optimizer/{runtime_environment.py,session/manifest.py,assets/check_magpie.py},
src/hyperloom/common/{benchmark_config.py,perf_metric.py,agentx_workload.py,env_safety.py},
src/hyperloom/orchestrator/actions/executors/{magpie_agentx.py,benchmark_config.py,_workload_envs.py,_grid_runner.py,_grid_base.py,baseline.py,profile.py,benchmark_result.py,_accuracy_gate.py,integrate_patch.py,_integrate_attempt.py,_kv_metrics.py},
src/hyperloom/orchestrator/phases/{geak_native_revalidation.py,geak_rebench.py,geak_adoption.py,kernel.py},
src/hyperloom/orchestrator/loop/{writeback.py,intent_router.py,proposals.py},
src/hyperloom/orchestrator/trace_analysis/{trace_selfcert.py,_trace_rank.py},
src/hyperloom/inference_optimizer/breakdown/reporters/cross_section.py, AGENTS.md, .importlinter,
docs/{components/magpie.md,reference/environment-variables.md,compatibility.rst}; symbols
native_launch_evidence, agentx_measurement, read_agentx_report, magpie_server_launch, effective_env,
runtime_environment, effective_argv, seed_native_geak_proxy, build_env_spec, constraint_fields,
CONSTRAINT_FIELDS, benchmark_backend, workload_identity, agentx_backend, native_agentx_veto,
_measurement_succeeded, materialize_benchmark_config, materialize_variant, apply_launch_changes,
build_agentx_workload_spec, agentx_client_script, geak_overlay_is_loadable, apply_runtime_override,
submission_valid. Enumeration scope: tree-wide git grep under src/ for every one of the 185
identifiers the diff adds, and for the nine deleted aiperf symbols -- not a walk of the diff's frame.
| Ran: bash .claude/skills/review-pr/fetch.sh 1922; the chain agentx_measurement ->
native_launch_evidence -> seed_native_geak_proxy driven directly on a Magpie-shaped report, twice and
independently (requested_server_env carries the variables while accepted_env comes back '' and
extra_envs is overwritten to PATH-only); ruff check --select C901 --config lint.mccabe.max-complexity=20 over the 50 changed non-test src/*.py files at head and at the merge
base -- 26 units over 20 on both sides and the only movements are downward
(ProfileExecutor.__call__ 66 -> 47, _run_once 56 -> 54), so the complexity ceiling is clear;
gh pr view 1922 --json isDraft,additions,deletions.
SKIPPED: pytest -- this reviewer is on Windows and cli/kb.py imports fcntl at module scope, so
the repo's suites cannot be collected here; the test-behaviour axis is therefore checked by reading
the added tests and by calling the production functions directly, not by running the suite. The full
matrix is green at the current head in CI.
| Base: a3fc0ba | Head: 26a5cfd
chaojhou
left a comment
chaojhou
left a comment
There was a problem hiding this comment.
PR #1922 -- Use Magpie's native AgentX contract for optimization
What it does: Hyperloom carried its own AgentX execution stack -- a shell AIPerf client, a phase gate, an installer/repair path and a corpus-rules mirror -- beside Magpie, which already owns server launch, replay, profiling and cleanup. This PR deletes Hyperloom's copy and makes Magpie the sole owner: HYPERLOOM_AGENTX=1 translates the ordinary workload flags into a native Magpie YAML, the resolved workload is archived as <session>/benchmark.yaml and re-read on resume, candidates are composed into benchmark.agentx.launch_overrides instead of into env and extra-args, and KEEP evidence comes from Magpie's benchmark_report.json validity verdict. Profile rounds become diagnostic-only, and reviewed GEAK source products route through a new revalidation phase. Read against Magpie at the pinned 6a1ee26f, the core contract holds: agentx_measurement is strictly more conservative than Magpie's own verdict, latency units are seconds on both sides, "a profile round can never justify a KEEP" is enforced independently by Magpie (benchmarker.py:615-623) and by Hyperloom (report.py:26-31), and Magpie's launch receipt carries both effective_env and runtime_environment (agentx_launch.py:549, :554), so report.py and geak_proxy.py each read a field Magpie writes.
Blocking issues: 4
- [free:mlperf-submission-veto-deleted] Deleting the
submission_validveto makes an interrupted or incomplete MLPerf agentic round a usable measurement [verified]
Problem:is_valid_measurementlost the block that refused a round whosesubmission_validwas False (basebenchmark_result.py:795-807). At head it special-casesmagpie_agentxand otherwise falls through tocompleted > 0(src/hyperloom/orchestrator/actions/executors/benchmark_result.py:792-803). The MLPerf backend this PR keeps still produces the verdict:map_mlperfwrites"submission_valid": not reasons, withinterruptedwhenstate == "interrupted"andincomplete_runwhencomplete is not True(src/hyperloom/inference_optimizer/agentx/mapping.py:123-131), and_merge_raw_resultstill copies it onto the measurement (benchmark_result.py:524). The MLPerf YAML sets onlybenchmark_script(_workload_envs.py:507) and noagentx:block, so its report takes the non-native branch and reaches this gate.mlperf_agentic_client.sh:240-257exits non-zero only when the harness itself fails, so a summary that records an interrupted or incomplete run is mapped and exits 0, andserved_complete_protocol-- consulted only on a non-zero exit (baseline.py:3849,_grid_runner.py:1815) -- never sees it. src/hyperloom/orchestrator/actions/executors/benchmark_result.py:792
Impact: withHYPERLOOM_AGENTX=1 HYPERLOOM_AGENTIC_BACKEND=mlperf, a round whose harness summary says interrupted or incomplete now counts as valid and can anchor the baseline or win a KEEP, silently. Running the head and base trees on{"output_throughput": 1234.0, "completed_requests": 500, "submission_valid": False, "submission_invalid_reasons": ["interrupted"]}: base returns False, head returns True. The description says the MLPerf backend retains its entrypoints and states no change to its validity rule.
Action: keep the verdict honoured for the backend that still produces it -- refuse a measurement whosesubmission_validis False before themagpie_agentxbranch -- or removesubmission_valid/submission_invalid_reasonsfrommap_mlperfand the breakdown readers, so no field is written that nothing enforces. - [free:native-agentx-power-unmeasured] A native AgentX session with a power budget stops right after its baseline [verified]
Problem:extract_benchmark_measurementreturnsagentx_measurement(report)(src/hyperloom/orchestrator/actions/executors/benchmark_result.py:636) before the measurement dict that calls_round_gpu_power(:674, the only call site), so a native measurement carries nogpu_power_by_gpu_w.power_veto_reasonfails closed on a missing per-card reading and returnspower_unmeasured(src/hyperloom/common/perf_metric.py:386-387).baseline_budget_stop_reasonruns that veto on the baseline itself and returnsbaseline_over_power_budget(src/hyperloom/orchestrator/measurement/baseline_budget.py:63-82), whichwriteback.py:2060turns into the session's stop reason. The launch-time guard written to prevent this,power_budget_unmeasurable_error, refuses multi-node sessions, scriptable frameworks and an unavailable sampler, but has no native-AgentX clause (src/hyperloom/inference_optimizer/cli/bootstrap.py:215-241). At base there was no early return, so AgentX rounds were stamped. src/hyperloom/orchestrator/actions/executors/benchmark_result.py:636
Impact:HYPERLOOM_AGENTX=1 hyperloom optimize ... --max-power-w 3000on a single-node SGLang session passes the launch guard, measures a full baseline, logs "baseline does not satisfy the power budget (power_unmeasured) ... reported no per-GPU power" and stops -- the outcome the guard's own docstring says it exists to prevent. Not stated in the description.
Action: stamp the measured power onto the native measurement before returning (call_round_gpu_poweron the native branch), or, if Magpie's managed launch leaves the sampler nothing to read, add a native-AgentX clause topower_budget_unmeasurable_errorso the session refuses at launch instead of after a baseline. - [D9]
remove_argsis applied locally and also forwarded to Magpie, where it is matched against a different argv and errors on zero matches [verified]
Problem:apply_launch_changesstrips the names from Hyperloom's ownappend_argsthroughcompose_server_args(..., remove_args=...)and then accumulates the same names intolaunch["remove_args"]for Magpie (src/hyperloom/orchestrator/actions/executors/magpie_agentx.py:58-68). Magpie appliesremove_argsto the recipe's canonical launcher argv and raises unless each name matches exactly one existing option (Magpieagentx_launch.py:245-252at the pinned6a1ee26f). A flag the session's accepted stack appended lives inappend_args, not in that argv. Driving headapply_launch_changeswith an accepted--cuda-graph-max-bs 256and thenremove_args=["--cuda-graph-max-bs"]yieldsappend_args: ["--mem-fraction-static", "0.85"]andremove_args: ["--cuda-graph-max-bs"]; feeding those overrides to Magpie'sapply_launch_argswith an SGLang launcher argv that does not carry the flag raisesremove_args must match exactly one existing option: --cuda-graph-max-bs. src/hyperloom/orchestrator/actions/executors/magpie_agentx.py:68
Impact:SKILL.md:1010-1013-- a file this PR edits -- still tells the Decide role to emit exactly this shape, with{"name": "remove_cuda_graph_max_bs", "remove_args": ["--cuda-graph-max-bs"]}as the worked example, andGridVariant.remove_argscarries it through (_grid_base.py:36). Every ablation of a flag the session or the operator appended is refused at server launch, so a documented candidate family cannot run on the native path and each attempt costs a round.
Action: forward to Magpie only the names that occur in the recipe's launcher argv, and keep the local strip for the names Hyperloom appended -- they are different operations on different argv and should not share one list. Add a test that drives an ablation of an appended flag throughmaterialize_variantand checks the overrides against Magpie'sapply_launch_args. - [X3] The loop doc tells operators to pass
--enable-conc-sweepunder AgentX, which now aborts the default backend at startup [verified]
Problem:docs/conceptual/optimization-loop.md:459(not touched by this PR) reads "Under AgentX the sweep is off unless--enable-conc-sweepis passed, since each rung is a 3600 s window and the session grades at a fixed CONC; SWEEP then records a disabled skip." On the default native backend (agentic_backenddefaults tomagpie),configure_benchmarknow raisesValueError("AgentX concurrency is fixed per session; create a session for each CONC")for that flag (src/hyperloom/inference_optimizer/cli/benchmark_config.py:106). The same validator newly rejects--isl,--osl,--profile-osl,--max-model-len,--reference-script,--quantize,--quantize-scheme, a framework other than sglang/vllm, more than one node, andISL/OSL/RANDOM_RANGE_RATIO/MAX_MODEL_LEN/EP_SIZEthrough--extra-env(:99-121); no in-tree operator doc or--helptext names these rejections. docs/conceptual/optimization-loop.md:459
Impact: an operator following the loop doc on a native AgentX session gets a startup abort, and the doc describes a SWEEP skip path native sessions never reach.
Action: correct that paragraph to say the sweep is unavailable on native AgentX and that another concurrency needs its own session, and list the rejected flags where an operator meets them (theoptimizehelp or the AgentX section ofSKILL.md).
Checked: Magpie at the pinned 6a1ee26f9053363d3750b43e3d5afb5d1c4ce84e (modes/benchmark/benchmarker.py server_launch/execution_owner/diagnostic_only and _managed_agentx, result.py agentx_metrics and the benchmark_valid/publishable verdict, agentx_launch.py validate_overrides/apply_launch_args/prepare_server_launch and the receipt fields, agentx_profile_config.py, config.py TorchProfilerConfig); Hyperloom at head -- agentx/report.py, agentx/geak_proxy.py, magpie_agentx.py, cli/benchmark_config.py, common/benchmark_config.py, runtime_environment.py, benchmark_result.py (is_valid_measurement, served_complete_protocol, extract_benchmark_measurement, _merge_raw_result, _round_gpu_power), common/perf_metric.py (power_veto_reason, constraint_veto_reason), measurement/baseline_budget.py, cli/bootstrap.py (power_budget_unmeasurable_error), agentx/mapping.py (map_mlperf), assets/agentx/mlperf_agentic_client.sh, _workload_envs.py (apply_agentx_switch, prepare_agentx_runtime), phases/geak_adoption.py and geak_native_revalidation.py with their wiring in kernel.py/intent_router.py/writeback.py, SKILL.md, docs/conceptual/optimization-loop.md | Ran: head-vs-base is_valid_measurement on an interrupted MLPerf measurement (base False, head True); head apply_launch_changes on an accepted flag plus its ablation, and Magpie's pinned apply_launch_args on the resulting overrides (ValueError); gh api reads of Magpie at the pin. Not run: the PR's own test suite, and no GPU execution -- the description already marks native GPU execution and a full Coordinator/GEAK session as unvalidated on this branch | Base: a3fc0ba | Head: 26a5cfd
…magpie-agentx-clean # Conflicts: # scripts/code_metrics_baseline.txt
|
Thanks @chaojhou — fixed all four. MLPerf rejects incomplete/interrupted runs again, AgentX keeps per-GPU power for budget checks, and removing an appended flag no longer causes a second removal in Magpie. The docs now list the unsupported flags and explain that each concurrency needs its own session. 823 focused tests and the pinned Magpie contract checks pass. Also resolved the main conflict. Native GPU and full optimization-loop validation are still pending. |
| ensure_eval_unbound_outputs_patched, | ||
| eval_probe_targets_exist, | ||
| ) | ||
| from .benchmark_config import prepare_benchmark_runtime |
There was a problem hiding this comment.
benchmark_config imports _workload_envs only inside prepare_benchmark_runtime. The return path back to _grid_runner then crosses a TYPE_CHECKING import in collaborator.py, so this is a static dependency cycle rather than an executable import cycle. Fresh-process imports and all seven architecture checks pass.
|
Hello @haofrank @chaojhou This also has instructions for running a torch trace folder checker that verifies the collected traces and its compatibility with TraceLens. You can use this to ensure TraceLens requirements on trace collection are satisfied. Please verify with above script before merging this PR on a vLLM and SGLang run of top model for AgentX. |
|
Thanks @devalshahamd for documenting the requirements. Could we address #1948 in a follow-up PR after this one merges? We'd use Magpie's existing capture-window controls for one 256-step capture, starting 300 seconds into the measured phase. Hyperloom would apply the updated TraceLens patches and prepare the SGLang kernel shape tool, reusing its existing profiling setup. The follow-up would include vLLM and SGLang runs verified with your checker. Would that split work for you? |
chaojhou
left a comment
chaojhou
left a comment
There was a problem hiding this comment.
PR #1922 -- Use Magpie's native AgentX contract for optimization
What it does: Hyperloom carried its own AgentX execution stack beside Magpie, which already owns server launch, replay, profiling and cleanup. This PR deletes Hyperloom's copy and makes Magpie the sole owner: HYPERLOOM_AGENTX=1 translates the workload flags into a native Magpie YAML archived in the session, candidates are composed into benchmark.agentx.launch_overrides, and KEEP evidence comes from Magpie's benchmark_report.json verdict. Old-AgentX compatibility is intentionally dropped. Three of the previous card's four items are fixed at this head. is_valid_measurement refuses submission_valid is False again before the native branch, and map_mlperf always writes a bool. The native branch now stamps per-GPU power: I traced Magpie at 6a1ee26f, InferenceX at 408c015 and its pinned AIPerf, and AIPerf writes <output_dir>/benchmark_*/aiperf_artifacts/logs/aiperf.log with the exact (profiling) started / (profiling) complete lines _resolve_scan_logs reads. The docs and --help list the native rejections, matching the validator. The fourth item, remove_args, is fixed for explore's appended-only case but not in general; see item 1.
Blocking issues: 2
- [D9] The
remove_argsfix decides "recipe option or appended option" from a launch receipt that is absent on the grid paths, so integrate_patch raises in Magpie and explore measures the wrong config [verified]
Problem:recipe_option_names(src/hyperloom/orchestrator/actions/executors/magpie_agentx.py:48-61) readslaunch_evidence.jsonbeside the config only when itsrecipe_digestmatches.run_gridbuilds variants from per-call base YAMLs:explore_base,integrate_patch,conc_sweep_baseandspecialist_rebench.with_envs.yaml. Evidence is persisted only intovariant_NN_*slots, so no receipt ever sits beside these YAMLs andmaterialize_variant(:224) always gets an empty set. That produces two failures. (a) integrate_patch applies the accepted stack'sbase_remove_argstwice: once throughmaterialize_benchmark_config(integrate_patch.py:4549) and again throughrun_grid(:4628). If the stack removed an option that only Hyperloom appended (an operator--server-argsflag ablated and KEEP'd in explore), the first pass strips it locally. The second pass no longer sees it inappend_args, forwards it inlaunch_overrides.remove_args, and Magpie'sapply_launch_argsraisesremove_args must match exactly one existing option. (b) In explore, ablating an option present both in the recipe argv and in the appended args (--cuda-graph-max-bs 512over the recipe's256) is treated as appended-only, line :90. It is removed fromappend_argsand not forwarded, so the server runs the recipe's256while the variant is recorded as the ablation. src/hyperloom/orchestrator/actions/executors/magpie_agentx.py:90
Impact: (a) Once such an ablation is KEEP'd, every later native integrate_patch benchmark fails before the server starts. (b) An explore KEEP or REVERT can be decided on a launch that is not the candidate, and integrate/baseline then replay a different stack than the one measured. Both reproduce on the head code fed into the pinned Magpieapply_launch_args.
Action: take the recipe's option set from one source, the accepted baseline's verified launch receipt, held on shared state rather than looked up beside each YAML, and pass it to every materialization. Apply the accepted stack once on the native path instead of in bothmaterialize_benchmark_configandrun_grid. Add a test that runs a KEEP'd appended-only ablation through integrate_patch and an in-both ablation through explore against Magpie'sapply_launch_args. - [C1] On the native path the AgentX prefix-cache-off baseline gate merged in #1884 can no longer refuse SGLang or vLLM [verified]
Problem: this PR merged main (f2c5081), which brought in #1884'srefuse_cacheless_agentx_baseline. For SGLang and vLLM that gate reads onlylaunch_evidence.observed_server_config(disable_radix_cache is True/enable_prefix_caching is False); only the ATOM branch readsrequested_server_args. On the native path,_attach_baseline_launch_evidencereturns throughnative_launch_evidence, which hardcodes"observed_server_config": {}(src/hyperloom/inference_optimizer/agentx/report.py:95), andbuild_launch_evidence, the only writer of that field, never runs. Native Magpie is the default AgentX backend, so the gate is dead for both main engines. src/hyperloom/inference_optimizer/agentx/report.py:95
Impact:--server-args "--disable-radix-cache"on SGLang, or--no-enable-prefix-cachingon vLLM, reaches Magpie'sappend_argsandeffective_argv. Neither flag is in Magpie'sPROTECTED_ARGSor golden-acceptance checks at6a1ee26f. The cold-cache baseline is accepted as the anchor, and any later candidate that restores the cache is credited with the cache as a gain. That is the failure #1884 was merged to stop, and main refuses this case today. Reproduced with the head code: both baselines staysucceededwith noerror_class. #1884's tests stay green only because they build evidence withobserved_server_configfilled in, which the native path never produces.
Action: let the gate judge the native launch. Eitherprefix_cache_off_evidencereadsrequested_server_args/effective_argvfor SGLang and vLLM as it already does for ATOM, ornative_launch_evidencefillsobserved_server_configfrom Magpie'sbenchmark_*/server.logthroughobserved_server_config_from_log. Add a test that feeds native-shaped evidence throughrefuse_cacheless_agentx_baseline.
Checked: the fix commits fa3b121, 68e0f8b and 0b41e0b, and the merge f2c5081; benchmark_result.py (is_valid_measurement, the native branch of extract_benchmark_measurement, _round_gpu_power); _subprocess_kill.py (_resolve_scan_logs, _scan_logs_increment, _RecorderFanout.note_client_phases, _build_round_recorders); _gpu_power.py and _kv_metrics.py phase tuples; magpie_agentx.py (recipe_option_names, apply_launch_changes, materialize_variant); _workload_envs.py materialize_benchmark_config; integrate_patch.py two-pass stack application; explore.py, _grid_runner.py _build_variant_yaml; agentx/report.py native_launch_evidence; _agentx_prefix_cache.py and its baseline.py call site; geak_adoption.py, geak_native_revalidation.py and kernel.py after the refactor; agentx/mapping.py map_mlperf; docs and --help; Magpie at 6a1ee26f (workspace.py, agentx_runtime.py, agentx_launch.py PROTECTED_ARGS / apply_launch_args / _validate_golden_launch); InferenceX 408c015 agentic replay.py and its pinned AIPerf (timing/phase/runner.py, config_defaults.py) | Ran: head code on the real nested Magpie layout with amd-smi mocked (power sampled only between (profiling) started and (profiling) complete, giving per-GPU readings); head apply_launch_changes / materialize_variant / materialize_benchmark_config on appended-only, recipe-only and in-both ablations fed into the pinned Magpie apply_launch_args (integrate_patch appended-only raises; explore in-both keeps the recipe value); head native_launch_evidence plus refuse_cacheless_agentx_baseline for SGLang and vLLM with the cache-off flag (not refused); head is_valid_measurement on an interrupted MLPerf result (refused); CI at 0b41e0b (all green). Not run: GPU execution and a full optimization loop, which the description already marks as pending | Base: 79cad92 | Head: 0b41e0b
|
|
||
| from __future__ import annotations | ||
|
|
||
| from hyperloom.orchestrator.actions.executors._workload_envs import prepare_grid_benchmark_config |
|
|
||
| from __future__ import annotations | ||
|
|
||
| from hyperloom.orchestrator.actions.executors._workload_envs import prepare_grid_benchmark_config |
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
PR #1922 -- Use Magpie's native AgentX contract for optimization
What it does: Hyperloom carried a second AgentX execution owner beside Magpie -- its own aiperf_client.sh, phase gate, capability preflight and runtime repair -- so server launch, replay validity and trace capture were implemented twice and could disagree. This PR retires that path and makes Magpie the only native AgentX owner: one workload YAML is bound per session (HYPERLOOM_AGENTX=1 generates it from the ordinary workload flags, or --benchmark-config supplies one), archived as benchmark.yaml, and candidates are expressed as benchmark.agentx.launch_overrides while Magpie's public benchmark_report.json becomes the sole measurement and validity source.
Blocking issues: 4
-
[free:native-profile-unpromotable] A native AgentX profile reports success and is then silently dropped by the promotion gate [verified]
Problem:ProfileExecutor._measurement_succeededwas overridden to accept a diagnostic round viaround_succeeded, and_run_native_profilereturnsstatus="succeeded"withmain_trace_path/trace_input_ready/profile_capturesfilled. ButWritebackCollaborator.is_promotable_resultstill gatesprofileonis_valid_measurement, and the new native branch there returns False for exactly this round. src/hyperloom/orchestrator/actions/executors/benchmark_result.py:796 against src/hyperloom/orchestrator/loop/writeback.py:896
Impact:_promote_profilenever runs for a native session, solast_profile_trace,last_profile_launch_evidence_path,last_profile_status,last_profile_workloadandlast_trace_analyzeare never updated.request_handlers.py:3738callsstate.last_profile_trace"the only source" for the fusion/kernel lane, so TraceLens analysis, roofline re-anchoring and the kernel phase run on an empty or stale trace while the executor reports the round succeeded. Nothing is logged.
Action: align the gate with the executor -- acceptround_succeededfor aprofiletask inis_promotable_result, and add the test that carries adiagnostic_onlynative profile result through it. -
[free:runner-type] The generated native YAML pins the real board as
runner_type, not the Magpie runner label [verified]
Problem:benchmark["runner_type"] = args.gpu_typewrites the probed board, whileargs.gpu_typeis deliberately the real GPU andos.environ["GPU_TYPE"]carries the runner label that_gpu_runner_typederived from it. src/hyperloom/inference_optimizer/cli/benchmark_config.py:159
Impact: on MI308X and MI325X,_gpu_runner_typemaps tomi300xandcli/__init__.py:2154prints "WARN: mi308x uses mi300x as Magpie runner_type (... Magpie has no sglang_mi308x.sh / vllm_mi308x.sh yet)" immediately followed by "Magpie runner : mi300x (will inject runner_type into Magpie YAML)" -- and then the YAML carriesrunner_type: mi308x, so Magpie resolves a launch script this repo states does not exist and the baseline cannot boot. The legacy path is unaffected:_workload_envs.py:1344takes the label fromGPU_TYPE.test_agentx_cli_config.py:88asserts onlymi355x, the one board where the two labels coincide.
Action: write_gpu_runner_type(args.gpu_type)and extend the test to a board where the two differ. -
[X3] The GPU-power documentation still describes the pre-PR measured-phase window [verified]
Problem: "only while the round is in its measured phase (from server ready, and from AIPerf's measured-phase line under AgentX, until the client exits or the eval starts)". Under native AgentX this diff maps server-ready tobootrather thanmeasured(_build_round_recorders(ready_phase=...)), so sampling no longer starts at server ready, and it adds_AGENTX_MEASURED_END_MARKERS = ("(profiling) complete",)with afinishedphase that ends the window before the client exits. docs/conceptual/optimization-loop.md:256 against src/hyperloom/orchestrator/actions/executors/_subprocess_kill.py:265,913,1108
Impact: the one paragraph that defines which samples entergpu_power_avg_wdescribes a different window than the code, for the backend the description says now carries measured per-GPU power into baseline and KEEP budgets. An operator reading it cannot tell whether a boot-phase reading is included. Single copy; the same file is edited by this PR for the conc-sweep paragraph.
Action: restate the window for native AgentX -- sampling starts at the measured-phase line only, and ends on(profiling) complete. -
[X5]
HYPERLOOM_BENCHMARK_CONFIGis an operator-reachable knob that reaches no documentation [verified]
Problem: the new variable is honoured straight from the ambient environment --source = getattr(args, "benchmark_config", None) or benchmark_config_path()-- so exporting it alone selects the native workload and turns on AgentX grading (confirmed:agentx_enabled()returns True with only the export set). It appears in no.md/.rst, includingdocs/reference/environment-variables.md, which this PR edits, and the description does not mention it. src/hyperloom/inference_optimizer/cli/benchmark_config.py:34, src/hyperloom/common/benchmark_config.py:15
Impact: X5's subprocess-handoff exemption does not apply, because the CLI reads it from the operator's environment rather than only writing it for its own children (_ray_runtime.py:441also forwards it to Ray workers).configure_benchmarkmeanwhile rejects the same name in--extra-envwith "Use --benchmark-config to select the workload YAML", so the two surfaces disagree about whether an operator may set it.
Action: document it indocs/reference/environment-variables.mdwith its unset default, or declare it internal in the description and make the CLI ignore an ambient value.
SKIPPED: V4 -- openprs.txt carries five # PARTIAL: lines (#1032 #1154 #1276 #1675 #1869 compared on their first 100 files only); #1924 overlaps 13 files including _workload_envs.py and cli/parser.py and was not compared in full. Conflicting in-flight work on those paths is therefore unchecked.
Checked: common/{benchmark_config,perf_metric,agentx_workload}.py | cli/{benchmark_config,init,parser,bootstrap,preflight}.py | inference_optimizer/{inferencex_checkout,runtime_environment,session/manifest}.py | agentx/{report,geak_proxy,runtime,preflight,deploy,mapping}.py | executors/{magpie_agentx,benchmark_config,benchmark_result,baseline,profile,explore,_grid_runner,_grid_base,_workload_envs,_subprocess_kill,_accuracy_gate,_agentx_prefix_cache,_gpu_power,integrate_patch,report}.py | phases/{geak_native_revalidation,geak_adoption,geak_rebench,kernel}.py | loop/{writeback,proposals,intent_router,dispatcher}.py | docs/{components/magpie,conceptual/optimization-loop,how-to/optimize,reference/environment-variables}.md | assets/{install.sh,check_magpie.py,configs/agentx_sglang.yaml} | Ran: agentx_measurement + is_valid_measurement on a diagnostic Magpie report (round_succeeded True / valid_measurement False / is_valid_measurement False); gpu_runner_type over mi300x,mi308x,mi325x,mi355x; agentx_enabled() with only HYPERLOOM_BENCHMARK_CONFIG exported; scripted reader-count over every new top-level symbol; head-tree sweep for AIPERF_CLIENT_SCRIPT / map_aiperf / CANONICAL* / _INFERENCEX_REF_DEFAULT / agentx.repair / _agentx_timeouts (no survivors) | Base: 79cad92 | Head: da37a3e
|
Correction to my review above: two of the four items do not meet the blocking bar and I am withdrawing them, and the scope of item 1 was overstated. Withdrawn — 3 ( Withdrawn — 4 ( Narrowed — 1 (native profile not promotable). The defect is real, but it does not affect the roofline path: Items 1 (as narrowed) and 2 ( |
Magpie stamps a profiled replay diagnostic_only, so a native AgentX profile is
never a valid measurement. ProfileExecutor already accepts the round on
round_succeeded, but is_promotable_result still gated the profile lane on
is_valid_measurement, so _promote_profile never ran and last_profile_trace was
never refreshed while the executor reported success.
runner_type must carry the Magpie runner label, not the probed board: Magpie
resolves {framework}_{runner_type}.sh and ships none for mi308x or mi325x, which
is why $GPU_TYPE already holds the derived value.
|
Pushed
Tests: |
Both blocking items are fixed on this branch in aba9556 and CI is green at that head; two of the four original items were withdrawn in a follow-up comment.
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
No blocking issues remain.
Both items from my earlier review are fixed in aba9556, and the other two were withdrawn as not meeting the blocking bar (see the correction comment above). CI is green at that head: 33 checks pass, 7 skipped, all 12 test shards success. The py3.10 shard totals went 25583 -> 25588, which is exactly the five new test items, so the added tests were collected and run rather than skipped.
Checked: the native/synthetic dispatch at all three executors (baseline materialize_benchmark_config / _after_materialize_config / _preflight_server_argv, _build_variant_yaml, ProfileExecutor); the KEEP path through native_agentx_veto and CONSTRAINT_FIELDS into current_best / baseline_perf; apply_launch_changes removal composition across two Explore rounds against the recipe digest; read_agentx_report's None-vs-{} consumers in the accuracy gate; native_launch_evidence reachability from both attach sites; the MAGPIE_REF / InferenceX pins across install.sh, cli/preflight.py and docs/components/magpie.md; a head-tree sweep for surviving references to the retired AIPerf symbols; and the prefix-cache baseline gate reading observed_server_config from the native launch evidence.
Ran: agentx_measurement + is_valid_measurement + is_promotable_result on a diagnostic Magpie report; _gpu_runner_type across mi300x/mi308x/mi325x/mi355x; agentx_enabled() against an ambient HYPERLOOM_BENCHMARK_CONFIG; a reader-count scan over every new top-level symbol.
Note for whoever merges: the head commit aba9556 is mine, so this approval is not independent for that commit. Native GPU execution and a full Coordinator/GEAK session remain unvalidated on this branch, as the description states.
…back-config-env Main's AgentX contract change (#1922) renamed the entry point the profile tests and the baseline YAML comments name, and re-exported _default_baseline_config from baseline.py. The tests and comments follow main; the alias stays removed, so the one test that used it imports default_baseline_config from _workload_envs. The baseline keeps main's entries, tightened with --update-baseline.
6 participants
Hyperloom uses Magpie's native AgentX execution with its familiar workload flags:
Hyperloom translates the resolved flags into Magpie YAML, archives
benchmark.yamlin the session, and injects--server-args/ serving--extra-envthrough Magpie's verified launch overrides.--benchmark-configremains optional for advanced recipe and profiling settings. Resume reuses the archived workload and rejects conflicting workload flags. Custom models require--docker-image; registered recipes can supply their image. Run Hyperloom inside the serving image: this path uses local execution.Magpie owns the server, InferenceX/AIPerf replay, profiling and cleanup. Hyperloom owns candidate generation, reviewed source/kernel patches, TraceLens analysis, and KEEP/REVERT. A session fixes its workload and CONC; only valid, unprofiled Magpie reports for that same workload can justify KEEP. GEAK results remain proposals until Critic review and canonical Magpie revalidation.
This replaces Hyperloom's AIPerf client, installer/repair path and phase-gate machinery. Related to #1675. Compatibility boundary:
HYPERLOOM_AGENTX=1selects native Magpie for new sessions; old AIPerf sessions and client-specific controls are not migrated. Ordinary benchmarks and the independent MLPerf backend retain their entrypoints. Initial native scope is single-node SGLang/vLLM, one concurrency per session; trace replay does not use CLI ISL/OSL.Magpie is pinned to
6a1ee26f9053363d3750b43e3d5afb5d1c4ce84e. Native AgentX uses Magpie's InferenceX resolver and default revision408c015be4b22d14c69518643609669405507077, including theinferencex-e2e/layout. Hyperloom's syntheticINFERENCEX_PATHno longer overrides that selection; explicitbenchmark.inferencex_pathorMAGPIE_INFERENCEX_PATHremain supported. Ordinary benchmark/MLPerf installation and preflight retain their compatible3d5581562f643f9bdeb8410cd924e2c70906c966revision and patches. Launch/resume documentation and an advanced YAML example are included.MLPerf still rejects interrupted or incomplete runs. Native AgentX carries measured per-GPU power into baseline and KEEP budgets; sampling follows AIPerf's measured replay, including outstanding responses. The operator docs list native AgentX's unsupported flags and require a separate session for each concurrency.
Accepted launch changes remain relative to the verified baseline recipe unless replacement was explicitly requested. Explore and patch revalidation apply accepted changes once, preserving unrelated recipe/operator settings across successive KEEPs. Appended-only removals are handled locally; shared recipe options still reach Magpie's removal validation. Native server-log observations now reach the existing prefix-cache baseline gate, and GEAK receives explicit environment assignments and unsets as well as Magpie's runtime snapshot. Packaged installs include the Magpie capability-check script.
Validation:
The large deletion is the retired AIPerf implementation and its implementation-specific tests. Its supported acceptance, rollback and profiling contracts are covered at the Magpie boundary. This PR addresses one concern: making Magpie the sole native AgentX execution owner.
Size exception: the total diff includes removal of the retired shell client and its tests. Switching callers and removing the old execution owner together completes the replacement. The
size-exceptionlabel covers total diff size only; all code-metrics limits remain enforced. InferenceX checkout policy is separated from preflight telemetry, and KEEP reuses its already-composed controls rather than rebuilding them.