Skip to content

fix(profiling): make TraceLens and dynolog survive real GPU traces - #172

Open
Cemberk wants to merge 1 commit into
cem/tracelens-dummy-testsfrom
cem/tracelens-gpu-fixes
Open

fix(profiling): make TraceLens and dynolog survive real GPU traces#172
Cemberk wants to merge 1 commit into
cem/tracelens-dummy-testsfrom
cem/tracelens-gpu-fixes

Conversation

@Cemberk

@Cemberk Cemberk commented Aug 13, 2026

Copy link
Copy Markdown

Summary

#170 proved the TraceLens and dynolog tools work without a GPU. Running them on
an actual AMD GPU node against real rocprofv3 and dynolog captures proved they
did not work with one. This is the fallout.

  • rocprofv3 JSON is not valid UTF-8. rocprofv3 copies HIP API
    const char * arguments into its JSON verbatim, so an argument that does not
    point at a string leaves raw bytes behind. TraceLens loads traces with orjson,
    which rejects the whole document, so twelve stray bytes cost the entire report
    on a 295 MB trace. The analyzer now detects undecodable bytes incrementally
    (no 295 MB read into memory) and analyzes a sanitized copy, leaving the
    original trace untouched.
  • The dynolog trigger was arguing with a flag that does not exist.
    dyno gputrace has no --fail-on-no-process in the pinned v0.5.0, and exits 0
    whether or not it matched a process. Every attempt therefore looked like "no
    PyTorch process registered yet", and the trigger retried a request dyno had
    refused, twelve times, before giving up with a misleading diagnosis. It now
    reads processesMatched out of the response, and stops immediately when dyno
    rejected the request rather than when the workload is merely slow to register.
  • The multi-rank collective report matched traces it could not use. It
    globbed the whole run directory and assumed every PyTorch trace carried its
    rank in its filename. Traces captured on demand are named after the process
    id, so TraceLens matched three files and rejected all of them. The report is
    now built only from rank-labelled traces, from a glob scoped to their own
    directory, and is skipped with an explanation when no trace carries a rank.
  • --python was not actually isolating anything. _build_command fell back
    to searching PATH for TraceLens console scripts, which defeats the isolation
    that flag exists to provide: TraceLens pins protobuf and xprof and lives in a
    venv of its own.
  • pip install -e . failed outright on this branch. The tracelens extra
    installs from git, and hatchling rejects direct references unless they are
    allowed explicitly. Nobody could install the branch to try any of the above.

Test plan

Every item above has a regression test that fails without its fix and needs no
GPU, so CI covers them. On the GPU node (banff-cyxtera-s83-5):

  • tests/e2e/test_tracelens_dummy_pipeline.py + test_dynolog_dummy_pipeline.py — 35 passed
  • tests/unit/test_tracelens_analyze.py + test_tracelens_report.py + tests/integration/test_tracelens_tools_config.py — 66 passed
  • tests/e2e/test_tracelens_workflows.py (GPU-gated) — 9 passed, 2 skipped
  • Real rocprofv3_lightweight + tracelens run end to end: the analyzer
    reported the trace as not valid UTF-8, sanitized it, and produced a
    SUCCESS row from the real TraceLens_generate_perf_report_rocprof
  • Real torch_profiler_dynolog capture, then TraceLens on the resulting
    Kineto trace
  • The whole tracelens or dynolog or profil surface: 127 passed. The three
    test_profiling_workflows.py failures (rpd, gpu_info_power,
    gpu_info_vram) are pre-existing and unrelated — they fail identically at
    ec4de0b, the commit before any TraceLens work. rpd dies inside the
    container on a missing librocm_sysdeps_sqlite3.so, and the gpu_info
    tools fail in gpu_info_pre.sh.

Note

Stacked on #170, so it targets cem/tracelens-dummy-tests.

Everything here came out of running the tools on an AMD GPU node against real
rocprofv3 and dynolog captures. None of it is reachable without hardware, which
is why the no-GPU test suite went green over broken code.

- rocprofv3 copies HIP API `const char *` arguments into its JSON verbatim, so
  an argument that does not point at a string leaves raw bytes behind. TraceLens
  loads traces with orjson, which rejects the whole document, so twelve stray
  bytes cost the entire report on a 295 MB trace. The analyzer now detects
  undecodable bytes incrementally and analyzes a sanitized copy, leaving the
  original trace untouched.

- `dyno gputrace` has no `--fail-on-no-process` flag in the pinned v0.5.0, and
  exits 0 whether or not it matched a process. Every attempt therefore looked
  like "no PyTorch process registered yet" and the trigger retried a request
  dyno had refused, twelve times. It now reads `processesMatched` out of the
  response, and gives up immediately when dyno rejected the request rather than
  when the workload is merely slow to register. `dynolog_stop.sh` reports that
  outcome instead of blaming the workload.

- The multi-rank collective report globbed the whole run directory and assumed
  every PyTorch trace carried its rank in its filename. Traces captured on
  demand are named after the process id, so TraceLens matched three files and
  then rejected all of them. The report is now built only from rank-labelled
  traces, from a glob scoped to their own directory, and is skipped with an
  explanation when no trace carries a rank.

- `_build_command` fell back to searching PATH for TraceLens console scripts,
  which defeats the isolation `--python` exists to provide: TraceLens pins
  protobuf and xprof, and is installed in a venv of its own.

- `pip install -e .` failed outright on this branch: the `tracelens` extra
  installs from git, and hatchling rejects direct references unless they are
  allowed explicitly.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI lite review requested due to automatic review settings August 13, 2026 01:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the profiling toolchain (TraceLens analyzer + dynolog integration) to behave correctly on real AMD GPU nodes, addressing invalid UTF-8 rocprofv3 JSON traces, dynolog request/response handling, and multi-rank collective report trace selection. It also updates packaging metadata so the tracelens extra can be installed from git under hatchling.

Changes:

  • Add incremental UTF-8 validation and “sanitized copy” analysis for rocprofv3 / PyTorch JSON traces, and keep --python isolation by avoiding PATH fallback.
  • Fix dynolog trigger semantics by interpreting processesMatched from output (exit code is not meaningful) and failing fast on rejected requests; improve stop-script diagnostics.
  • Restrict multi-rank collective report inputs to rank-labelled traces and add regression tests; enable hatch direct references for pip install -e ..

Reviewed changes

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

Show a summary per file
File Description
tests/unit/test_tracelens_analyze.py Updates collective-args expectations and adds coverage for skipping collective reports when ranks are not identifiable.
tests/fixtures/dummy_tracelens/TraceLens/Reporting/_dummy.py Makes the dummy TraceLens fail on invalid UTF-8 JSON to emulate orjson/TraceLens behavior.
tests/e2e/test_tracelens_dummy_pipeline.py Adds e2e regression tests for analyzing traces containing undecodable bytes via sanitized copies.
tests/e2e/test_dynolog_dummy_pipeline.py Expands the dyno stub contract and adds tests for fail-fast behavior on rejected requests and accurate reporting.
src/madengine/scripts/common/tools/tracelens_analyze.py Implements incremental UTF-8 detection + sanitization, improves collective report trace selection/globbing, and enforces --python environment isolation.
src/madengine/scripts/common/tools/dynolog_trigger.sh Switches from non-existent flag semantics to parsing processesMatched, and fails fast when dyno rejects the request.
src/madengine/scripts/common/post_scripts/dynolog_stop.sh Improves post-run messaging to distinguish “no process matched” vs “request rejected”.
pyproject.toml Enables hatchling allow-direct-references to support git-based dependencies in extras.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +310 to +313
if workspace[0] is None:
workspace[0] = tempfile.mkdtemp(prefix="madengine-tracelens-")
destination = os.path.join(workspace[0], os.path.basename(trace))
print(
Comment on lines +556 to +564
elif mode in ("auto", "collective") and len(pytorch_traces) > 1:
unrankable.append(
(
f"{len(pytorch_traces)} PyTorch traces",
"the collective report needs the rank in each trace's filename, "
"and none of these carry one. Traces captured on demand through "
"dynolog are named after the process id.",
)
)
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.

2 participants