Skip to content

env probe: recover Buck-target torch native libs + capture triton/fbgemm commits (schema 1.8)#226

Open
vivekkhandelwal1 wants to merge 2 commits into
mainfrom
env-probe/schema-1.8-buck-libtorch-recovery-and-pkg-commits
Open

env probe: recover Buck-target torch native libs + capture triton/fbgemm commits (schema 1.8)#226
vivekkhandelwal1 wants to merge 2 commits into
mainfrom
env-probe/schema-1.8-buck-libtorch-recovery-and-pkg-commits

Conversation

@vivekkhandelwal1

Copy link
Copy Markdown
Collaborator

Summary

Two recoverability gaps surfaced while confirming aorta env probe against a Buck torch target (fbcode//caffe2:torch), where import torch succeeds in-process but the Python package is a link-tree and the C++ runtime is dlopen'd from a build-artifact dir:

  • Buck/monorepo native-lib recovery. libtorch_hip.so was located only at <torch.__file__>/../lib, which does not exist in a Buck par layout, so composable_kernel.pytorch_bundled, aotriton.*, and pytorch_build.binary_introspection.* came back null. Added a /proc/self/maps fallback (_loaded_lib_path_from_maps / _torch_native_lib_dir) that recovers the real lib directory from the process's mapped libraries (torch is already imported). The original on-disk path and reason strings are preserved as the last-resort fallback, so existing "missing lib" diagnostics are unchanged when nothing is locatable. Linux-only, fail-soft.
  • Package commit capture. Generalized aiter's +g<sha> parse into _extract_commit_from_version / _capture_python_package_commit and added a commit field to the triton and fbgemm blocks. Best-effort: setuptools_scm +g<sha>, ROCm fork .git<sha>, or a git_version/__commit__ module attr; null when no SHA (e.g. "3.5.0+fb").

Schema 1.7 → 1.8, additive nested keys only — EnvSnapshot.from_dict still round-trips pre-1.8 snapshots. Updates docs/env-probe.md.

Test plan

  • tests/instrumentation/test_environment.py — added TestPackageCommitExtraction and TestTorchNativeLibDir (incl. a test proving the symbol dump recovers via /proc/self/maps for a Buck-style torch with no <torch>/lib); updated the 3 existing triton/fbgemm assertions + schema-version pin.
  • env + probe e2e + recipe suites pass (one unrelated pre-existing failure: test_partial_false_on_clean_full_probe, which requires triton installed in the venv and fails identically on main).
  • Live validation against a real ROCm libtorch_hip.so on a gfx950 Buck torch target (unit-tested with a fake mapped lib; not yet exercised against a live HIP build).

Made with Cursor

…emm commits (schema 1.8)

Two recoverability gaps surfaced while confirming the probe against a
Buck torch target (fbcode//caffe2:torch), where torch imports in-process
but its Python package is a link-tree and the C++ runtime is dlopen'd
from a build-artifact dir:

- libtorch_hip.so was located only at <torch.__file__>/../lib, which
  does not exist in a Buck par layout -> composable_kernel.pytorch_bundled,
  aotriton.*, and pytorch_build.binary_introspection came back null. Add a
  /proc/self/maps fallback (_loaded_lib_path_from_maps / _torch_native_lib_dir)
  that recovers the real lib dir from the process's mapped libraries, since
  torch is already imported. Original on-disk path + reason strings are kept
  as the fallback-of-last-resort, so existing diagnostics are unchanged when
  nothing is locatable. Linux-only, fail-soft.

- Generalize aiter's +g<sha> commit parse into _extract_commit_from_version /
  _capture_python_package_commit and add a `commit` field to the triton and
  fbgemm blocks (best-effort: setuptools_scm +g<sha>, ROCm fork .git<sha>, or
  a git_version/__commit__ module attr; null when no SHA, e.g. "3.5.0+fb").

Schema 1.7 -> 1.8 (additive nested keys only; from_dict still round-trips
pre-1.8 snapshots). Adds unit tests for commit extraction and the maps-based
lib recovery; updates docs/env-probe.md.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings June 16, 2026 10:40

Copilot AI 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.

Pull request overview

Improves aorta env probe recoverability and metadata capture when running against Buck-target PyTorch builds by (1) finding torch native libraries via /proc/self/maps when <torch>/lib is absent, and (2) capturing best-effort git commits for triton and fbgemm_gpu into the env snapshot (schema bump 1.7 → 1.8).

Changes:

  • Bump env snapshot schema version to 1.8 and add nested commit fields under triton and fbgemm.
  • Add /proc/self/maps fallback to locate torch native shared libraries for Buck/monorepo layouts.
  • Update unit tests and documentation to reflect the new schema and recovery behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/aorta/instrumentation/environment.py Adds commit extraction helpers, adds triton.commit/fbgemm.commit, and introduces /proc/self/maps fallback for torch native lib discovery.
tests/instrumentation/test_environment.py Updates schema pin and assertions; adds new tests for commit parsing and Buck torch native-lib recovery.
docs/env-probe.md Documents schema 1.8 additions and the Buck/monorepo native-lib recovery behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/aorta/instrumentation/environment.py
Comment thread docs/env-probe.md
@vivekkhandelwal1

Copy link
Copy Markdown
Collaborator Author

Live-validated on a gfx950 node against a real ROCm libtorch_hip.so (torch 2.9.1+rocm6.4, torch.version.hip=6.4.43484-..., 371 MB lib):

  • maps locator found the loaded libtorch_hip.so from /proc/self/maps.
  • Primary path (real <torch>/lib): symbol counts populated (pytorch_flash::=4, _efficient_attention=18, aotriton::=3, mha_fwd_aot=1), torch_lib_bundled={libaotriton_v2.so: true}, aotriton bundled_version=0.11.0 with real hash — confirms the refactor doesn't regress the normal layout.
  • Fallback path (bogus <torch>/lib, lib still mapped — i.e. the Buck-target scenario): _torch_native_lib_dir recovered the real lib dir via /proc/self/maps, the nm | c++filt dump processed 30,482 demangled symbols, no "not found" reasons, and symbol counts were identical to the primary path.
  • triton commit: this ROCm wheel reports package_version=3.5.1 with no SHA → commit: null (correctly no false-positive).

So the previously-null composable_kernel.pytorch_bundled / aotriton.* / pytorch_build.binary_introspection.* fields now populate for a Buck torch target. Checking the live-validation box.

…cs to 1.8)

- _capture_python_package_commit no longer returns an arbitrary module
  attribute value: a commit attr (git_version/__commit__/...) is accepted
  only if it embeds or is a 7-40 char hex SHA, via the new
  _commit_from_attr_value helper. Non-SHA values ("unknown", "dirty", a
  tag) now yield None so the `commit` field is always a real SHA or null.
- docs/env-probe.md: schema_version row + sample output bumped 1.7 -> 1.8,
  and added the 1.8 changelog entry (demoting 1.7 from "current").
- Tests: reject non-SHA attrs, accept/lowercase a bare full SHA, and a
  unit test for _commit_from_attr_value.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@amd-vivekag amd-vivekag left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Verdict: ship (two Low nits, nothing blocking)

Clean, well-scoped change. The two mechanisms — /proc/self/maps recovery of torch's native lib dir for Buck layouts, and best-effort commit extraction for triton/fbgemm — are both fail-soft, Linux-guarded, and don't touch the common wheel path (so no perf/behaviour regression when <torch>/lib exists). Schema bump is genuinely additive: only nested commit keys added, from_dict still round-trips pre-1.8, and _disaster_snapshot + the example snapshot were both updated to match. Test coverage is strong, including the key Buck-recovery case proving nm gets pointed at the maps-recovered path and no "not found" reason is emitted.

Re-review of prior Copilot comments — both Fixed (correct):

  • Non-SHA module attr leaking into commit: resolved via _commit_from_attr_value (validates embed-or-is a 7-40 hex SHA), with tests rejecting unknown/dirty/tag and accepting+lowercasing a bare SHA. Verified.
  • Docs still on 1.7: schema_version row, sample output, and changelog all bumped to 1.8. Verified.

Findings: 0 High · 0 Medium · 2 Low

Nice touch: preserving the original <torch>/lib path in the fallback reason string so the "missing lib" diagnostic still names a concrete path.

parts = line.split()
if len(parts) < 6:
continue
path_str = parts[5]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Low] line.split() + parts[5] truncates any mapped path that contains a space (e.g. /opt/my libs/libtorch_hip.so becomes /opt/my), so the recovery silently misses the lib.

The pathname is always the trailing field, so split with a bounded count and take the remainder:

Fix:

Suggested change
path_str = parts[5]
parts = line.split(maxsplit=5)
if len(parts) < 6:
continue
path_str = parts[5]

(the len(parts) < 6 check just above can be dropped since it's now folded in; adjust as needed). Real-world impact is small (torch libs rarely live under spaced paths), hence Low.

_COMMIT_SHA_RE = re.compile(r"[0-9a-f]{7,40}", re.IGNORECASE)


def _extract_commit_from_version(version: str | None) -> str | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Low] The PR body says aiter's +g<sha> parse was "generalized into _extract_commit_from_version", but _capture_aiter still has its own inline re.search(r"\+g([0-9a-f]{7,40})", version) (~line 3432). So there are now two divergent commit parsers, and aiter's is the weaker one (it won't catch the .git<sha> fork form this new helper handles).

Fix: have _capture_aiter call _extract_commit_from_version(version) instead of its own regex, or drop the "generalized" wording from the PR description. Not blocking — aiter's format is +g<sha> so it still works today.

amd-vivekag pushed a commit that referenced this pull request Jul 6, 2026
…t, schema 1.9)

Adds three top-level blocks to `aorta env probe`, deepening the static
capture of on-disk, runtime-selected GPU-compute catalogs and clearly
labeling them as installed-library identity (the "recipe book") -- NOT
the runtime-selected solution/solver ID (381881/368xxx), which needs a
workload trace and stays out of scope for the no-compute probe.

- tensile_catalog (issue #54): reuses the hipblaslt/rocblas/tensile
  captures (no regression) + a per-file `menu` enumeration of the frozen
  Tensile solution library (TensileLibrary/*.dat/*.yaml/*.co/*.hsaco):
  per-file content sha256 + size, logic_file_count, gfx_arch_coverage,
  and combined_content_hash, so a two-host diff localizes WHICH logic
  file differs (the 7e32d53eb1 "same commit, different install" case).
- miopen_catalog (#54 follow-up): same treatment for MIOpen's databases
  under /opt/rocm/share/miopen/db/ (.kdb/.fdb.txt/.db.txt/.db selectable
  DBs + .ktn.model/.tn.model heuristic nets); honors MIOPEN_SYSTEM_DB_PATH.
- rocfft_catalog (#54 follow-up): fingerprints rocFFT's optional AOT
  rocfft_kernel_cache.db when present; absence is the common case
  (status="absent", no partial reason).

Partial-not-silent: a dir that can't be located/listed is "partial"
with a reason (distinct from a present-but-empty menu, which is "ok"
with logic_file_count=0). All three are additive defaulted dataclass
fields, so older readers drop the unknown keys and from_dict back-fills.

Schema 1.7 -> 1.9 (1.8 reserved for PR #226, expected to land first).
Verified on a real ROCm 7.0.2.2 MI350 node: tensile_catalog enumerates
1485 hipBLASLt + 505 rocBLAS logic files across 11-12 gfx arches,
miopen_catalog 27 logic DBs, rocfft_catalog absent; full collect_env
~4-6s (no GPU compute). 384 env-probe unit tests pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

3 participants