Repository navigation
refactor(inference-optimizer): pass trace, roofline, multi-node and CLI inputs as typed objects instead of parallel keywords - #1973
Conversation
lishuoshuo-amd
added
the
size-exception
Code metrics gate: PASSEDJudged: the 72 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): 5
Already over at the base (backlog, not this change's): 1775 findings in files this change does not touchOutside this change (informational): 75
...and 50 more; the job log lists every row.
Thresholds and sources: |
d142a79 to
a371e4a
Compare
PR #1973 -- refactor(inference-optimizer): pass trace, roofline, multi-node and CLI inputs as typed objects instead of parallel keywordsWhat it does: In Blocking issues: none Checked:
Ran:
Step 7 independent reader: a second reader re-checked the rebase seam in the three shared files and the gate totals (397 -> 347 function-args rows) at head. LGTM |
Every KB span nests under the unphased agent span.
…one tool call as ToolCall record_event builds its row inline instead of forwarding every argument to _build_row.
…on and its task as _ConfirmTarget
…tGeometry to the ceiling formulas The SDPA and fused-MoE helpers take one _SdpaShape / _MoeShape; _sdpa_bytes drops the causal flag it never read.
…g carrying its RooflineBreakdown
…caller passes The digest always reads the mode from HYPERLOOM_AGENTX.
…k_only into check_only The installer entry point still accepts both --check-only and --dry-run.
…alyze one TargetAnalysisRequest
…eptedMetrics record
The ssh helpers drop the user nobody overrides; the Infera wrappers drop the remote_path and env their callers never pass.
…tor's server-log sweep
… status predicates Every caller passed the same terminal-status checks and none passed the failure diag or quiet-404 grace.
…rds to the RayJob patch and restart helpers _submit_and_collect_pod_json drops the runtime_env and success_statuses no caller passes.
…he kill-only argv into build_node_kill_args The pid/log paths, dist-init port and health settings no caller overrides become module constants.
…as _FanoutOptions
The 45s per-card fallback no caller overrides becomes _GPU_FALLBACK_TIMEOUT_S.
…ts _PdRole and _LaunchDirs The launch summary reads the group leaders from _GroupHeads.
…ting and one CriticSetup
…nPromptContext The phase re-scope partial binds the context; the action_registry override no caller passes is dropped.
LaunchInfo and _emit_launch_info sit with the other session bootstrap printers in cli/bootstrap.py.
…atorLaunchShape _reexport_session_environment takes the session id it reads instead of the whole manifest.
…equest and the PartitionModel it must fit
…nish the event from one PreflightRuntime
…ptimizer signature refactor
a371e4a to
4c9d8b9
Compare
|
Rebased onto main after #1972 and #1974 (head 4c9d8b9). The code commits applied without conflicts; only the code-metrics baseline was regenerated. Re-verified at this head: code-metrics gate passes, the test files of the four files shared with #1972 pass (916 tests), every changed module imports on Python 3.10 and 3.11, and CI is green (33 pass, 7 skipped). The earlier LGTM stands. |
chaojhou
left a comment
chaojhou
left a comment
There was a problem hiding this comment.
PR #1973 -- refactor(inference-optimizer): pass trace, roofline, multi-node and CLI inputs as typed objects instead of parallel keywords
What it does: brings inference_optimizer outside breakdown/ under the parameter-count limit. 50 of 54 functions now take the record their values belong to (SshTarget / RemoteScript, NodeLaunch, KillWaits, the pod launcher's server group, ComputeTarget / DecodeShape / DitGeometry, LLMCallSite, EventWindow, OrchestrationPromptContext, ModelRouting, InstallStep and others). Parameters no caller passes are deleted, the job-status predicate all 7 _short_poll callers repeated moves inside it, and framework_deps.ensure's duplicate dry_run folds into check_only. No CLI flag, launcher argv, persisted journal or log line changes.
Blocking issues: none
Non-blocking (both are the PR's own "delete what no caller passes" left one step short; small to finish here):
WorkloadTerminalFailure(src/hyperloom/inference_optimizer/multi_node/cli.py:81) and itsexceptbranch inmain()(:1844-1848) can no longer run: this PR deleted_short_poll(failure_diag=), which held the onlyraise. TheEXIT_TERMINAL_FAILUREcomment (:76) and themain()docstring (:1836, "2 terminal workload failure (do not retry)") describe a path nothing produces; a FAILED job raisesRuntimeErrorand exits 1. Exit 2 andMULTI_NODE_FAILURE_SNAPSHOTstill come fromlaunch_multinode.pyand the PD refusal at :1438, so deleting the class and branch and re-scoping the comment and docstring to those removes no live path._infera_ssh_bash_with_envlostenv, and both its calls (cli.py:254, :263) now pass a literalNonetossh_run_bash_with_env, whoseenvparameter,exportprologue andassert_env_key_shapesonly a test still reaches. Droppingenvthere (ssh_run_bash) and renaming the wrapper finishes the deletion instead of moving theNonedown a layer.- Smaller:
RemoteScript.remote_pathandDitGeometrybeing a NamedTuple are kept only for tests (the test file also adds an adapter rebuilding the old 8-keyword signature);compute_compute_bound_ceiling_tok_per_secstill takes threeModelMetafields unpacked by both callers while its sibling takesModelMetawhole;multi_node/_internal/__init__.py:4still says the CLI is the only public surface thoughmulti_node/__init__.pynow exportsSshTarget.
Checked: every production hunk in the 40 files for field-to-keyword mapping and defaults; the constants that replace deleted parameters (build_node_launch_args ports, pid/log paths and health wait; _GPU_FALLBACK_TIMEOUT_S; KillWaits); build_node_kill_args; the 7 _short_poll callers; Journal.load_or_create merge; resume-guard confirmation rows; LLMCallRecord sites and record_event callers (no ts= / start_ts= left, including via **context); framework_deps flags; workload_signature digest; callers of every deleted parameter across src, scripts, docs and the Ray pod scripts; docs and both SKILL.md files for removed names | Ran: base vs head on build_node_launch_args / kill args, the SSH helpers' subprocess.run argv, input and timeout, an emulated Ray launch_multinode.py (sglang/vllm, aggregated and disaggregated), roofline ceilings over 2 models x 3 GPUs x 3 shapes x 3 precisions x 2 GPU counts, build_roofline_snapshot, tool_attributes, LLMCallRecord rows, render_reference_script and gap_analysis (all identical); CI green at head. Not run: pytest (not installed here), real Ray or SSH | Base: d914a83 | Head: 4c9d8b9
LGTM
|
What it does 50 functions in Blocking issues: none. What was checked
Not checked: the Base LGTM |
Rebased onto main after #1970, #1971, #1972 and #1974; the files it shares with #1970 (
kernel/conc_sweep.py,knowledge/recipe_journal.py,roles/critic_agent.py) and #1972 (knowledge/recipe_journal.py,loop/coordinator.py,state/shared_state.py,test_resume_guard.py) merged without conflicts, and both sides' shapes stand.Description: what and why
Brings
src/hyperloom/inference_optimizeroutsidebreakdown/under the parameter-count limit (function-args> 5). For each function the question was where the values come from and where they travel together; each now takes the object they belong to:LLMCallSite(call identity forLLMCallRecord.from_metadata/for_failure),ToolCall,EventWindow.Journal.load_or_createtakes the freshJournalits caller describes; the resume guard chain carries_Confirmationand_ConfirmTarget.ModelMetaandRooflineBreakdownpass them whole;DecodeShape,ComputeTarget,DitGeometry(a NamedTuple, so tuple unpacking keeps working),_MoeShape,_SdpaShape,RooflineReading.LaunchSettings,ServingTarget,BaselineQuery(callers already built it),TargetAnalysisRequest,AcceptedMetrics.SshTarget/RemoteScriptfor the SSH helpers (published frommulti_node/__init__.pyso the orchestrator's server-log sweep does not reach into a private module),KernelPatchApply,_MultinodeLaunch,_FanoutOptions,NodeLaunch(the kill-only mode ofbuild_node_launch_argsis split out asbuild_node_kill_args),KillWaits,_ServerGroup/_PdRole/_LaunchDirs/_GroupHeadsfor the pod launcher._short_pollkeeps the job-status predicates all 7 callers passed identically inside the function.OrchestrationPromptContext,LaunchInfo(moved with_emit_launch_infointocli/bootstrap.py, which keepscli/__init__.pyfrom growing),PartitionRequest/PartitionModel,_OperatorLaunchShape,ModelRouting/CriticSetup,InstallStep,PreflightRuntime.framework_deps.ensurehaddry_runandcheck_onlydoing the same thing; they are one parameter now (both CLI flags still exist).No
**kwargs, untyped dict, compatibility shim or new fallback branch is added; every call site is updated.Scope: 54 functions over the limit; 50 are done. 4 keep their signatures because a caller lives in an orchestrator file another open PR is restructuring:
recipe_canonical_idandkb_hardware_slug(knowledge/recipe_kb_t0.py),inject_sglang_context_length(actions/executors/_workload_envs.py), andcanonical_labels, which sharesrecipe_canonical_id's seven identity dimensions and moves with it.Gate count of functions over the parameter limit (
function-argsentries inscripts/code_metrics_baseline.txt): 256 on main -> 206 (-50); the cyclomatic-complexity entries and the broad-exceptcount are unchanged.Linked issue(s): none.
Tests: added/updated? commands run?
_sdpa_bytesignores itscausalargument is removed with that argument.--helpis byte-identical between main and this head forhyperloom,optimize,recover,check,multi-nodeand all 9 of its subcommands, andframework_deps. The pod kill / launch scripts needrayfor--help; no argparse line changed in them.python:3.10-slimandpython:3.11-slim(two need a third-party package absent from the slim image).PYTHONPATH=src(never the full suite): the 31 changed test files (1591 passed) and 34 caller / related test files (1406 passed, 2 skipped).ruff check/ruff format --checkon the changed files;python scripts/code_metrics.py --base-ref origin/mainPASSED (parameter count -50); baseline lowered with--update-baseline.Size/complexity triggers crossed: split, or why the shape is right:
Diff budget: 2503 production Python lines changed (+1166 / -1337 in 40 production
.pyfiles). One concern (parameter count of the inference_optimizer modules outside breakdown and their call sites). Each commit touches at most 5 production files and is grouped by module. The code-metrics gate passes without a waiver.If this simplifies or refactors: which mechanism was removed, which contract was preserved, and the tests that show it:
Removed: the parallel keyword lists, the duplicated job-status predicates at the 7
_short_pollcall sites, and the deaddry_runduplicate ofcheck_only. Preserved: every CLI flag, default and help text (above), launcher argv and env, persisted journals and confirmations.Deleted parameters (checked across src, scripts, docs and tests, including aliases):
record_kb_span(phase=),ssh_*(user=),_infera_ssh_run_script(remote_path=),_short_poll(failure_diag=, quiet_fetch_error_grace_s=, is_quiet_fetch_error=),_submit_and_collect_pod_json(runtime_env=, success_statuses=),build_node_launch_args(dist_init_port=, pid_file=, log_file=, health_port=, health_wait_sec=)(now constants with the old defaults),_kill_remote(gpu_fallback_timeout_s=),_build_orchestration_prompt(action_registry=),workload_signature(benchmark_mode=): no caller passes them._infera_ssh_bash_with_env(env=): its only caller passesNone._sdpa_bytes(causal=): never read.Observable effect: what an operator will now see, and what the old behaviour cost them (if none, why the change is unobservable):
None for valid inputs: no CLI flag, output file, command line or log line changes. Two edge cases: the multi-node kill-only path no longer parses
state["nodes"], so a state file with a malformed node count no longer fails a kill; andWorkloadTerminalFailurecan no longer be raised, which was already true in practice because no caller passed a failure diag (its exit-code handler is left in place).Breaking changes: yes, for Python callers only: the refactored functions take their typed objects. Every in-tree caller is updated.
PR addresses single concern: yes (parameter count of
inference_optimizeroutsidebreakdown/).Root cause is upstream (Magpie/TraceLens/GEAK/IntelliKit/AgentKernelArena), ticket filed: n/a.