src/aiu_trace_analyzer/pipeline/rcu_utilization.py: average PT active…#83
Open
tirtha-ibm wants to merge 1 commit intoIBM:mainfrom
Open
src/aiu_trace_analyzer/pipeline/rcu_utilization.py: average PT active…#83tirtha-ibm wants to merge 1 commit intoIBM:mainfrom
tirtha-ibm wants to merge 1 commit intoIBM:mainfrom
Conversation
tirtha-ibm
requested review from
WarningRan,
eklee15,
lasch,
mcalman and
yuhaohaoyu
as code owners
… information with autopilot on in torch profiler Signed-off-by: Tirtha Biswas <Tirtha.Biswas@ibm.com>
lasch
requested changes
Feb 6, 2026
Member
lasch
left a comment
lasch
left a comment
There was a problem hiding this comment.
Thanks for your contribution. A few comments to address please:
- currently it's failing the flake8 test with an unused global variable
- there are more flake8 issues reported:
./src/aiu_trace_analyzer/pipeline/flow_launch.py:71:9: E265 block comment should start with '# '
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:353:11: E271 multiple spaces after keyword
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:702:1: E305 expected 2 blank lines after class or function definition, found 1
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:704:1: C901 'compute_utilization' is too complex (18)
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:704:1: E302 expected 2 blank lines, found 0
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:710:121: E501 line too long (140 > 120 characters)
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:731:13: F824 `global jobhash_map` is unused: name is never assigned in scope
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:753:5: E303 too many blank lines (2)
./src/aiu_trace_analyzer/pipeline/rcu_utilization.py:758:16: E111 indentation is not a multiple of 4
- additional comments below
This is a welcome addition and feature. I'm happy to work this out directly.
| else: | ||
| aiulog.log(aiulog.WARN, "FLOWS: Ignoring event with ts after schedule wait", event) | ||
| assert self.queues[qid]["last_ts"] <= sched_wait_end, f"{qid}: {event}, {self.queues[qid]}" | ||
| #assert self.queues[qid]["last_ts"] <= sched_wait_end, f"{qid}: {event}, {self.queues[qid]}" |
Member
There was a problem hiding this comment.
Is there a reason for commenting out this check? Did you encounter traces where it failed?
| _autopilot_pattern = re.compile(r'DSM-AutoPilot BEGIN') | ||
| _iteration_mode_pattern = re.compile(r'^\s+(DECODING|PREFILL)\s+$') | ||
| _non_kernel_names = ["Total"] | ||
| _total = re.compile(r'Total ') |
Member
There was a problem hiding this comment.
Please follow the pattern of the other variable names and use _total_pattern
Comment on lines
+702
to
+703
| jobhash_map: dict[int, bool] = {} | ||
| processID: int = 0 |
Member
There was a problem hiding this comment.
Global variables should be avoided here. The context classes should be used to hold 'global' state for the functions of the pipeline.
Comment on lines
+727
to
+738
| if event["name"] != "aiuScheduleWait": | ||
| return [event] | ||
| try: | ||
| ideal_dur = float(context.get_ideal_dur(kernel_name, pid, job_fingerprint)) | ||
| if autopilot and event["name"] == "aiuScheduleWait": | ||
| global jobhash_map | ||
| if event["args"]["jobhash"] not in jobhash_map: | ||
| ideal_dur = list(totals)[1] | ||
| jobhash_map[event["args"]["jobhash"]] = True | ||
| else: | ||
| ideal_dur = list(totals)[0] | ||
| else: | ||
| ideal_dur = float(context.get_ideal_dur(kernel_name, pid, job_fingerprint)) |
Member
There was a problem hiding this comment.
This functionality should be baked into the context. I suggest the following rough idea if autopilot is enabled:
- we add the 'total' cycles to the corresponding table structure as a 'mock-up' kernel name: e.g. 'supernode_kernel'
extract_kernel_from_event_namereturns the 'supernode_kernel' as the name to lookup- the rest should then fall into place automatically
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… information with autopilot on in torch profiler