Repository navigation
perf: move numeric manager term params to runtime buffers instead of plan_key constants - #36
Conversation
…plan_key constants Numeric tuning arguments (termination thresholds, observation scales, reward gains, ...) were repr-baked into the generated kernel source and the plan key, so every tuning variant triggered a full ~1.2-30s kernel recompilation. Lower float term args into a fixed float32 shared input slot instead: only the layout fingerprint enters the plan key and the value is read at runtime, keeping tuning variants on one compiled plan. Reward weights are no longer pre-multiplied by ctrl_dt at build time; the dt scaling happens inside the evaluate kernel via ctx.dt, removing ctrl_dt from the plan key and the double-baking mismatch risk. Also improve startup logging: the manager env now logs each startup step (build plan, compile kernels with cache hit/miss classification, term specializations, warmup execution) with durations, and the runner logs each training startup phase. Kernel cache detection covers both numba cache layouts (NUMBA_CACHE_DIR set/unset), and fastsac worker processes configure INFO logging so startup timings are visible. Fixes #34
Scalar term args now lower into one runtime buffer source each, so the wbt read plan legitimately grows beyond the hardcoded count of four. The test's intent (preallocated, stable kernel inputs with the manager context present) is preserved without pinning the source count.
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness/robustness gaps (a weakened test assertion and subprocess logging setup that may no-op under preconfigured handlers) that should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR reduces Numba manager kernel recompilation for tuning-only config variants by lowering float scalar term parameters into fixed np.float32 runtime input buffers (so only layout enters plan_key, not numeric values), and by removing ctrl_dt from the plan key via moving dt scaling into the fused evaluate kernel. It also adds more granular startup/compile timing logs and updates/adds tests to validate plan reuse and correct runtime semantics.
Changes:
- Lower float-like scalar term args into a shared runtime buffer (
TermScalarBuffer) to avoid baking numeric values into generated source /plan_key. - Remove
ctrl_dtfromplan_keyby applyingctx.dtscaling inside the generated reward accumulation code. - Add startup timing/cache-classification logs across manager env startup and RL runner/fastsac subprocesses; update/add tests for plan reuse and semantics.
File summaries
| File | Description |
|---|---|
| motrix_rl/src/motrix_rl/runner.py | Adds per-stage training startup timing logs around backend resolution, run context creation, and handle readiness. |
| motrix_rl/src/motrix_rl/fastsac/async_impl/worker.py | Configures spawned worker-process logging so INFO startup logs are visible from collector/learner processes. |
| motrix_envs/tests/test_wbt_numba.py | Adjusts a plan/read-path reuse test to accommodate additional runtime scalar-buffer sources. |
| motrix_env_core/tests/test_numba_manager.py | Updates reward weight expectation (no longer pre-multiplied by dt) and adds a plan-sharing test across scalar/ctrl_dt variants. |
| motrix_env_core/src/motrix_env_core/numba/manager/env.py | Refines manager env startup logging to be step-based with timing and completion markers. |
| motrix_env_core/src/motrix_env_core/numba/manager/compiler/compiler.py | Implements TermScalarBuffer, shifts float scalar args to runtime buffers, removes dt from plan key, and adds build/compile/specialization timing + cache-classification logging. |
| motrix_env_core/src/motrix_env_core/numba/manager/compiler/codegen.py | Applies ctx.dt scaling inside the generated weighted reward accumulation. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…assertion - Worker logging setup no longer relies on basicConfig when the root logger already has handlers: raise the root level instead so startup INFO records surface through a preconfigured setup. - The wbt read-plan test now concretely asserts the presence of runtime scalar-buffer sources (TermScalarBuffer) instead of only leaving a comment, so regressions that bake scalar args back into the plan or generated source fail the test.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core manager-kernel compilation/caching semantics (plan key, runtime inputs, cache invalidation) and should receive final human validation for regressions/perf across environments.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
背景
#32 之后 manager kernel 按
plan_key在进程内去重,但一些纯数值型配置量(terminationthreshold、obsscale/gain、reward/obs 裸标量参数等)仍以repr(value)烘进生成源码与 plan_key,导致调参型变体各自触发一次全额 kernel 重编译。 solves #34。改动
1. 标量 term 参数 runtime buffer 化(#34 优先级 1)
np.float32的 shared 输入槽(TermScalarBuffer,@kernel_data):布局指纹进 plan_key,数值运行时读取。np.float32,与 fused kernel 内联特化签名一致。2.
ctrl_dt移出 plan_key(#34 优先级 2)task.reward_weights存原始权重,dt 缩放移入 evaluate kernel 内reward * weight * ctx.dt,消除manager_context_dt与 reward weights 编译期相乘的双重烘入。3. 启动日志
in-process/disk/miss(miss 时预告首次编译耗时)。NUMBA_CACHE_DIR设置与否),_invalidate_generated_cache同步修正;顺带修复默认布局下 fallback 清理无法删除缓存文件的问题。runner.create_training_handle各阶段(backend 解析、run context、trainer 就绪)带耗时日志。性能评测(g1-wbt-dance, 4096 envs)
端到端 step 无可测回退;调参变体的编译开销从线性增长变为 0。
测试
test_scalar_term_args_and_ctrl_dt_share_one_compiled_plan:不同 scale/threshold/ctrl_dt 变体共享同一 plan_key 且数值语义各自正确。task.reward_weights断言(原始权重,不再预乘 dt)。motrix_env_core/tests128 项全部通过;prek run通过。Fixes #34