Skip to content

Conversation

@FindHao
Copy link
Member

@FindHao FindHao commented Oct 26, 2025

Summary

This PR fixes a critical bug in the reproducer functionality where the --line CLI argument was not properly converted from 1-based user input to 0-based array indexing, causing users to reproduce the wrong kernel launch event.

Problem

The reproducer's --line parameter was documented as 1-based (where line 1 is the first line), but the implementation was using it directly as a 0-based array index:

Before:

  • User runs: tritonparseoss reproduce trace.ndjson --line 1
  • Expected: Process the 1st launch event (index 0)
  • Actual: Process the 2nd launch event (index 1) ❌

This mismatch meant users were consistently reproducing the wrong kernel launch event.

Root Cause

In tritonparse/cli.py, the CLI argument args.line was passed directly to reproduce() without conversion:

reproduce(
    input_path=args.input,
    line_index=args.line,  # BUG: No conversion from 1-based to 0-based
    ...
)

The internal implementation in ndjson.py uses 0-based indexing:

launch_event = events[line_index]  # 0-based array access

Changes Made

1. Fixed CLI Conversion (tritonparse/cli.py)

reproduce(
    input_path=args.input,
    line_index=args.line - 1,  # Convert 1-based line number to 0-based index
    ...
)

2. Updated Documentation

  • orchestrator.py: Clarified that line_index parameter is "0-based index"
  • ingestion/ndjson.py: Updated docstrings for get_launch_and_compilation_events() and build_context_bundle() to specify "0-based index"
  • README.md: Fixed Python API example from line_index=1 to line_index=0 with clear comment

Behavior After Fix

CLI (1-based for user convenience)

tritonparseoss reproduce trace.ndjson --line 1  # First launch event
tritonparseoss reproduce trace.ndjson --line 2  # Second launch event

Python API (0-based, Pythonic)

reproduce(input_path="trace.ndjson", line_index=0)  # First launch event
reproduce(input_path="trace.ndjson", line_index=1)  # Second launch event

Testing

The fix maintains consistency with existing test code which already uses 0-based indexing:

# From tests/test_tritonparse.py
launch_indices = [i for i, ev in enumerate(events) if ev.get("event_type") == "launch"]
line_index = launch_indices[0]  # 0-based index
reproduce(..., line_index=line_index, ...)

Impact

  • Users: CLI now works as documented - --line 1 correctly processes the first launch event
  • API: No breaking changes - Python API continues to use 0-based indexing (Pythonic convention)
  • Documentation: Now consistent across CLI help text, docstrings, and README examples

Files Changed

  • tritonparse/cli.py - Added conversion logic
  • tritonparse/reproducer/orchestrator.py - Updated docstring
  • tritonparse/reproducer/ingestion/ndjson.py - Updated docstrings
  • README.md - Fixed example code and comments

Update README example to use 0-based indexing for reproduce.line_index, adjust CLI to convert user-provided line (1-based) to a 0-based index (args.line - 1), and clarify docstrings in ndjson and orchestrator to state line_index is 0-based.
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 26, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 27, 2025

@FindHao has imported this pull request. If you are a Meta employee, you can view this in D85571489.

@meta-codesync
Copy link

meta-codesync bot commented Oct 27, 2025

@FindHao merged this pull request in a4cc7e4.

@FindHao FindHao deleted the fix-reproducer-line-index-1based-to-0based branch October 30, 2025 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants