Skip to content

Support: record torch backend autoload state - #1977

Open
Crane-Liu wants to merge 1 commit into
hw-native-sys:mainfrom
Crane-Liu:codex/record-torch-backend-autoload-1950
Open

Support: record torch backend autoload state#1977
Crane-Liu wants to merge 1 commit into
hw-native-sys:mainfrom
Crane-Liu:codex/record-torch-backend-autoload-1950

Conversation

@Crane-Liu

Copy link
Copy Markdown
Contributor

Summary

  • Record the effective torch backend autoload configuration and observed torch/torch_npu module state before the first SceneTest dispatch.
  • Emit one TIMING record per SceneTest process across L2 and L3 execution, including forked processes.
  • Make bind-phase comparisons auditable from their logs and document the record in the measurement workflow.

Testing

  • Pre-commit hooks
  • Python unit tests: 1911 passed, 11 skipped
  • Full a2a3sim SceneTest resource and L2 runtime phases
  • Full a5sim SceneTest resource and L2 runtime phases
  • SceneTest smoke with backend autoload disabled
  • Queued real-environment smoke with backend autoload at its default

Fixes #1950

Emit one TIMING record per SceneTest process after torch-dependent
argument preparation. The record captures the effective backend-autoload
configuration and whether torch and torch_npu are loaded, so performance
logs carry their own measurement context.

Add L2/L3 lifecycle and PID-aware once coverage, and document the record
in the bind-phase comparison workflow.

Fixes hw-native-sys#1950
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4090d380-af92-41a4-9390-d7f01d6a6c65

📥 Commits

Reviewing files that changed from the base of the PR and between 969c6a6 and a46135c.

📒 Files selected for processing (3)
  • docs/dfx/hbg-bind-phases.md
  • simpler_setup/scene_test.py
  • tests/ut/py/test_scene_test_torch_autoload.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The scene-test harness records Torch backend autoload settings and module state once per process. L2 and L3 emit the record before execution. Tests verify state reporting, process behavior, and timing. Measurement documentation uses the record for branch comparisons.

Changes

Torch autoload diagnostics

Layer / File(s) Summary
Autoload state recording
simpler_setup/scene_test.py, tests/ut/py/test_scene_test_torch_autoload.py
The harness classifies TORCH_DEVICE_BACKEND_AUTOLOAD, inspects Torch module state, and emits one timing record per process. Tests cover normalization, module state, non-import behavior, and process re-emission.
Execution timing integration
simpler_setup/scene_test.py, tests/ut/py/test_scene_test_torch_autoload.py
L2 and L3 emit the autoload record after setup and before worker execution. Tests verify execution ordering and L3 resource release.
Measurement guidance
docs/dfx/hbg-bind-phases.md
The recipe documents the timing record and requires matching autoload conditions before branch comparison.

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

Merge Risk: ⚪ Minimal · up to a4613

This localized change records torch backend autoload state and documents the measurement workflow; no actionable merge-blocking risk remains after normal checks and review.

Poem

I’m a rabbit with logs in my den,
Checking Torch once, then once again.
L2 hops first, L3 follows the trail,
Matching records keep measures on scale.
“No hidden autoload!” I cheer with delight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: recording Torch backend autoload state.
Description check ✅ Passed The description directly explains the diagnostic record, process behavior, documentation updates, testing, and linked issue.
Linked Issues check ✅ Passed The changes satisfy issue #1950 by recording effective autoload settings and observed module state without changing runtime behavior.
Out of Scope Changes check ✅ Passed The code, tests, and documentation changes are directly related to the linked issue objectives and contain no unrelated scope.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.

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.

@ChaoWao

ChaoWao commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Review — a46135ce (merge base 969c6a67)

All 19 checks pass. 3 files, +225/−6 = 231.

  Core             1 files  +   34  -    1  =35    simpler_setup/scene_test.py
  Test/Ex          1 files  +  181  -    0  =181   tests/ut/py/test_scene_test_torch_autoload.py
  Docs             1 files  +   10  -    5  =15    docs/dfx/hbg-bind-phases.md
  TOTAL            3 files  +  225  -    6  =231

What it does

One TIMING line, emitted once per process just before the first dispatch:

torch_backend_autoload setting=0 effective=disabled torch_imported=true torch_npu_loaded=false

setting is the raw env classification (unset / 0 / 1 / invalid), effective is
torch's own interpretation, and the two sys.modules booleans are the observed outcome
the issue asked for. Purely diagnostic — no behaviour gate, and it correctly does not
set TORCH_DEVICE_BACKEND_AUTOLOAD=0 itself, which #1950 explicitly deferred.

Two things it gets right that were the actual risk in this change:

The effective mapping is faithful. I read the installed torch 2.7.1 rather than
assuming: _is_device_backend_autoload_enabled() is
os.getenv("TORCH_DEVICE_BACKEND_AUTOLOAD", "1") == "1". So unset → enabled, "1"
enabled, and everything else including "0", "true", garbage → disabled. The PR's
raw is None or raw == "1" is exactly that, including the non-obvious part (=true
means off). A wrong mapping here would make the audit line lie, which is worse than
having no line.

The placement is where it has to be — after generate_args, _build_l2_ref_args
and compute_golden, before the rounds loop (scene_test.py:1692), and after
_RehostedTaskArgs on the L3 side (:1777). torch_interop imports torch lazily inside
_ensure_torch_map(), so torch enters sys.modules during arg build; logging any earlier
would report torch_npu_loaded=false for a run that in fact paid for it. Both call sites
are inside _run_and_validate_l2 / _run_and_validate_l3, which are the only two
branches of _run_and_validate — so scene-test dispatch coverage is complete, not sampled.

The test file is the strong part: it asserts the exact message text for all four env
classes, both module states, once-per-process dedupe, and — the two that matter —
call-site ordering (events == ["args", "record", "run"] and
["rehost", "record", "run", "release"]), so a future edit that tidies the call upward
fails a test rather than silently corrupting the record.

Verified locally: 10 passed; ruff clean; pyright 0 errors. The suite-wide 1911 passed
I can't reproduce here — 37 collection errors from an unbuilt _task_interface in my
worktree, unrelated to the diff, and CI's ut passes on both platforms.


Should explain

1. "including forked processes" — no forked process reaches either call site. The
pid-keyed @cache exists specifically so a fork re-emits, but tracing the forks:

  • python/simpler/worker.py:7761,7795,7869 — the os.fork() children enter
    _chip_process_loop, whose own signature comment says the child "cannot read parent
    state after os.fork". They run runtime worker loops, never _run_and_validate_*.
  • The multi-process scene-test fanout (_dispatch_test_phases_standalone, the conftest
    dispatcher) is sys.executable script … — a fresh interpreter with an empty cache, so
    it emits once without needing a pid key.

So nothing in the tree exercises the fork branch, and
test_record_is_emitted_again_in_a_forked_process tests a mechanism that has no caller.
The dedupe that is needed is across multiple cases in one interpreter, which a
module-level flag gives with less machinery — and the _process_id parameter name
(leading underscore, conventionally "unused") actively hides that it is the cache key.
Either name the fork path that re-enters, or simplify to a flag and drop the claim from
the summary.

Consider

2. Pin the torch source in a comment. effective replicates a private torch
predicate. If torch relaxes the comparison, accepts "true", or flips the default, this
line reports the wrong effective state and nothing fails — the record is diagnostic, so
there is no oracle. One comment naming torch._is_device_backend_autoload_enabled() and
its getenv(..., "1") == "1" shape makes that a ten-second re-check instead of a
re-derivation.

3. setting=invalid discards the value. For a record whose entire purpose is
auditability, two logs launched with =true and =yes read identically. And "invalid" is
a judgement torch doesn't share — =true is deterministically off, which effective=
already states. Emitting the raw value (truncated, since it's unbounded env input) costs
nothing and keeps the audit complete.

4. logging.getLogger("simpler") is load-bearing and unexplained. It's the only such
call in simpler_setup/ outside log_config.py, and the file's own
logger = logging.getLogger(__name__) sits 70 lines above. That module logger's effective
level is root's WARNING, so logger.log(TIMING, …) drops the record entirely — I
confirmed logging.getLogger("simpler_setup.scene_test").isEnabledFor(25) is False
after configure_logging("timing"). Credit where due: I also confirmed the tests catch
the swap (with caplog.set_level(TIMING, logger="simpler"), only the "simpler"-logger
line is captured), so this is reader-cost, not risk. One clause on the .log( line.

5. The ordering guard has a small hole. Both ordering tests pass skip_golden: True,
so compute_golden never appears in events. Moving the call to between
_build_l2_ref_args and compute_golden would keep every test green while making
torch_npu_loaded reflect a pre-golden state. Theoretical today — arg build imports torch
first — but adding golden to the asserted sequence, or one present-tense line stating
the record must follow the last torch-importing step, closes it for free.

6. Pre-existing, in the table this PR edits. Two rows above the one it rewrites,
SIMPLER_LOG_LEVEL=TIMING names a variable nothing reads: the only non-doc hit is a
string inside an error message in hbg_bind_phases.py:110, and log_config.py's own
docstring says "there is no env var; the Python simpler logger is the single source of
truth." The new record does show up in that recipe — but because --log-level defaults to
timing, not because of that row. Not this PR's bug; worth one line while the table is
open, since the new sentence depends on TIMING records being emitted there.

ℹ️ pto-isa pin: cd4a3d3f…, unchanged; no pto-isa header references touched —
advisory only.

Verdict

Approve. It does exactly what #1950 asked and declines what #1950 declined. The one
property that had to be right — matching torch's actual env semantics, including the
counter-intuitive =true → off — is right, checked against the installed torch. Placement
is correct and, unusually, guarded by ordering tests rather than a comment. The single
substantive item is that the fork-handling machinery and the summary's claim about it
don't correspond to any fork this repo makes; everything else is a comment or a log-field
improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Health] Nothing records whether a run paid torch_npu's device autoload

2 participants