Skip to content

Adding prefill decode markers to trace and enable shapes - #156

Closed
msiddaiah wants to merge 3 commits into
mainfrom
atom_model_tracing
Closed

msiddaiah wants to merge 3 commits into
mainfrom
atom_model_tracing

Conversation

@msiddaiah

Copy link
Copy Markdown

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

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

This pull request adds profiler markers to distinguish between prefill and decode phases in PyTorch profiler traces, and introduces a new environment variable to enable shape recording independently of other detailed profiling features.

Changes:

  • Added ATOM_PROFILER_SHAPES environment variable to control shape recording in the profiler
  • Added nested profiler markers in run_model() to mark prefill/decode phases, model forward passes, and CUDA graph replays
  • Added top-level profiler markers in forward() to mark the entire forward pass phase

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

Comment thread atom/model_engine/model_runner.py Resolved
Comment thread atom/model_engine/model_runner.py Resolved
Comment thread atom/model_engine/model_runner.py Outdated
Comment on lines +958 to +959

with torch_profiler.record_function(f"forward_pass_{phase_marker}"):

Copilot AI Jan 20, 2026

Copy link

Choose a reason for hiding this comment

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

The profiler marker naming is inconsistent between forward() and run_model(). In forward(), the markers are named "forward_pass_PREFILL" and "forward_pass_DECODE", while in run_model() they use "PREFILL_PHASE" and "DECODE_PHASE" with additional suffixes like "_model_forward" or "_cudagraph_replay_bs{graph_bs}". Consider using a consistent naming scheme across both methods for easier trace analysis, such as using "PHASE" suffix consistently or removing it from run_model().

Suggested change
with torch_profiler.record_function(f"forward_pass_{phase_marker}"):
with torch_profiler.record_function(f"{phase_marker}_PHASE"):

Copilot uses AI. Check for mistakes.
Comment thread atom/model_engine/model_runner.py Resolved

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@valarLip
valarLip requested a review from HaonanWang98 January 21, 2026 02:55
@valarLip

Copy link
Copy Markdown
Collaborator

thanks your contribution, @HaonanWang98 had a more powerful work on the way, i would l like that one

Copilot AI review requested due to automatic review settings January 22, 2026 21:35

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 1 out of 1 changed files in this pull request and generated 2 comments.


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

logits = self.run_model(input_ids)
# Add top-level marker for the entire forward pass
is_prefill = batch.total_tokens_num_prefill > 0
phase_name = "PREFILL" if is_prefill else "DECODE"

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

Trailing whitespace detected at the end of this line. Please remove the trailing spaces after the variable assignment.

Suggested change
phase_name = "PREFILL" if is_prefill else "DECODE"
phase_name = "PREFILL" if is_prefill else "DECODE"

Copilot uses AI. Check for mistakes.

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@zufayu

zufayu commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Closing due to inactivity: no pushes, reviews, or comments for 207 days. This is housekeeping, not a judgement on the change — if you want to continue this work, comment here and we'll reopen, or open a fresh PR rebased on the latest main.

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.

4 participants