Skip to content

chore(runtime): bump simpler to 4d5fbe4c; adapt STRACE span rename (supersedes #1907)#1933

Closed
luohuan19 wants to merge 3 commits into
hw-native-sys:mainfrom
luohuan19:feat/bump-runtime-4d5fbe4c
Closed

chore(runtime): bump simpler to 4d5fbe4c; adapt STRACE span rename (supersedes #1907)#1933
luohuan19 wants to merge 3 commits into
hw-native-sys:mainfrom
luohuan19:feat/bump-runtime-4d5fbe4c

Conversation

@luohuan19

Copy link
Copy Markdown
Contributor

Problem

Bumping the runtime submodule to current simpler main (4d5fbe4c) requires two pypto adaptations, one of which fails silently:

  1. simpler [Bug] pl.cast(index -> UINT) drops type annotation in pto.tci, PTOAS fails with 'expected :' #1181 dropped arg_index from CoreCallable.build (build(signature, binary)), while pre-[Bug] pl.cast(index -> UINT) drops type annotation in pto.tci, PTOAS fails with 'expected :' #1181 runtimes still require it. pypto passes arg_index= unconditionally → TypeError on [Bug] pl.cast(index -> UINT) drops type annotation in pto.tci, PTOAS fails with 'expected :' #1181+ runtimes.
  2. simpler refactor(codegen): Lower tile.slice/assemble to pto.subview #1210 renamed the [STRACE] span-tree root run_preparedsimpler_run (device emitters + strace_timing._ROUNDS_TABLE_NAMES). pypto's bench.py hardcoded the old names in _parse_stats_from_strace, so on a refactor(codegen): Lower tile.slice/assemble to pto.subview #1210+ runtime by_name().get(...) returns None and every host/device wall sample is silently 0.0 (no error).

Fix

Adaptation audit (02bd0c4f → 4d5fbe4c)

The rest of pypto's consumed surface is unchanged:

  • Public Worker API — refactor(codegen): Lower tile.slice/assemble to pto.subview #1210 renamed only internal _ChipWorker/binding methods (prepare_callableregister_callable), not Worker.register/run/....
  • Tensor struct + orchestration API headers — comment-only diff; host_api.h additions are internal.
  • simpler_setup tools consumed by pypto (torch_interop, strace_timing, swimlane_converter, log_config, KernelCompiler) — signatures intact.

Testing

  • tests/ut/runtime/test_benchmark.py17 passed against the 4d5fbe4c runtime (parse tests exercise the real strace_timing, resolving to simpler_run). The fixture now sources the span root from _ROUNDS_TABLE_NAMES, keeping the tests generation-agnostic.
  • ruff clean.
  • test_benchmark_st.py (asserts device_wall_us > 0) is the on-device regression guard for the span rename — not yet run on-device here.

Notes

No public API change. Distributed L3/L2 and SPMD paths are not device-validated post-#1210 (larger separate surface).

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da79dd16-9f4d-41b1-b4b0-3f0a18bf94e4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR updates benchmark span parsing in bench.py to derive host/device span names dynamically from strace_timing._ROUNDS_TABLE_NAMES (replacing hardcoded run_prepared naming with simpler_run), adds a CoreCallable.build compatibility shim for arg_index in device_runner.py, bumps the runtime submodule pin, and updates related tests.

Changes

Runtime span rename and build compatibility

Layer / File(s) Summary
Dynamic span key derivation
python/pypto/runtime/bench.py
Removes hardcoded _SPAN_HOST/_SPAN_DEVICE constants, derives host_key/device_key from _ROUNDS_TABLE_NAMES at parse time, and updates docstrings/notes to reflect the simpler_run span naming.
Test suite updates for dynamic span_root
tests/ut/runtime/test_benchmark.py
Replaces require_simpler fixture with a span_root fixture, generalizes synthetic marker generation, and updates parse/format/mean-tree assertions accordingly.
CoreCallable.build compatibility shim
python/pypto/runtime/device_runner.py
Adds _BUILD_ACCEPTS_ARG_INDEX detection and a _build_core_callable helper that conditionally passes arg_index, wiring both cached and newly-compiled binary paths through it.
Runtime submodule bump
runtime
Updates the pinned submodule commit SHA.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A span renamed, a key derived,
run_prepared no more survived,
With arg_index tucked away just right,
Old and new builds both take flight,
Hop hop, submodule pinned anew —
This rabbit's tests all still pass through! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the runtime bump and STRACE span rename adaptation.
Description check ✅ Passed The description directly explains the runtime bump and the two compatibility fixes in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces compatibility support for multiple generations of the simpler runtime by dynamically sourcing span names and detecting the presence of the arg_index parameter in CoreCallable.build. The review feedback suggests robust error handling for cases where older runtime versions lack the expected metadata, and a fallback mechanism for when Python is run with optimized settings that strip docstrings, which would break the current feature detection.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread python/pypto/runtime/bench.py
Comment thread python/pypto/runtime/device_runner.py Outdated
Comment thread tests/ut/runtime/test_benchmark.py
noabauma and others added 3 commits July 3, 2026 11:00
…w-native-sys#1181

simpler hw-native-sys#1181 ("replace arg_index with positional dump + func_id array")
removed the ``arg_index`` parameter from ``CoreCallable.build`` — the binding
is now ``build(signature, binary)``. pypto passes ``arg_index=`` and therefore
raises ``TypeError`` against hw-native-sys#1181+ runtimes, while pre-hw-native-sys#1181 runtimes still
*require* ``arg_index`` parallel to ``signature``, so it cannot be
unconditionally dropped.

Feature-detect ``arg_index`` support once from the nanobind signature
(``CoreCallable.build.__doc__``) and pass it only when accepted, via a new
``_build_core_callable`` helper shared by both kernel-compile paths in
``compile_and_assemble``. pypto then drives both the pinned (pre-hw-native-sys#1181) and the
latest (hw-native-sys#1181+) simpler runtime without a pin bump.
Bumps the runtime submodule 02bd0c4f -> 4d5fbe4c (simpler main). The only
pypto-facing change across that range is simpler hw-native-sys#1210, which renamed the
[STRACE] span-tree root run_prepared -> simpler_run (in the device emitters
and strace_timing._ROUNDS_TABLE_NAMES; the host/device keys are unchanged).

bench.py hardcoded the old span names in _parse_stats_from_strace, so on a
hw-native-sys#1210+ runtime by_name().get(...) returned None and every host/device wall
sample was silently 0.0. Derive the names from the installed runtime's
_ROUNDS_TABLE_NAMES instead of hardcoding, so benchmark() works against both
runtime generations (matching the arg_index feature-detect approach).

test_benchmark.py: the parse tests built synthetic markers with a hardcoded
run_prepared root; the fixture now returns _ROUNDS_TABLE_NAMES["host"] and the
markers build their names off it, keeping the tests generation-agnostic.

The rest of pypto's consumed surface is unchanged across the bump: public
Worker API (hw-native-sys#1210 renamed only internal ChipWorker/binding methods),
CoreCallable.build's dropped arg_index (hw-native-sys#1181, feature-detected in the prior
commit), and the Tensor struct / orchestration API headers.
…s#1181+

The runtime submodule now pins to a post-hw-native-sys#1181 simpler (bumped in the
prior commit), where CoreCallable.build's arg_index parameter is gone.
Since pypto's runtime is the submodule via editable install, pin and
runtime are always the same generation, so the pre-hw-native-sys#1181 branch of the
feature-detect was dead the moment the pin crossed hw-native-sys#1181.

Remove the compat shim:
- device_runner: drop the __doc__-sniffing _BUILD_ACCEPTS_ARG_INDEX and
  the _build_core_callable helper; call CoreCallable.build(signature,
  binary) directly at both kernel-compile sites.
- pto_backend: stop baking "arg_index" into the generated
  kernel_config.py — nothing reads it post-hw-native-sys#1181 (runtime switched to a
  positional dump + func_id array), so it was write-only dead data with
  a now-false comment.

The unrelated dump-tag "arg_index" JSON field (runtime tensor-dump
schema) is untouched.
@luohuan19 luohuan19 force-pushed the feat/bump-runtime-4d5fbe4c branch from 5391dc1 to bcae731 Compare July 3, 2026 03:03
@luohuan19 luohuan19 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants