Skip to content

Separating result processor out from profiler.py - #23251

Merged
Jian Chen (jchen351) merged 11 commits into
mainfrom
Cjian/profiler
Jan 7, 2025
Merged

Separating result processor out from profiler.py#23251
Jian Chen (jchen351) merged 11 commits into
mainfrom
Cjian/profiler

Conversation

@jchen351

Copy link
Copy Markdown
Contributor

Description

Separating result processor out from profiler.py without changing the behaviors of current profile.py

Motivation and Context

Less dependency and smaller code for processing profile from other scenarios.

@github-actions github-actions Bot 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.

You can commit the suggested changes from lintrunner.

Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py
Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py
Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py Outdated
Comment thread onnxruntime/python/tools/transformers/profiler.py Fixed

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

Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py
Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py

@github-actions github-actions Bot 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.

You can commit the suggested changes from lintrunner.

Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py

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

Comments suppressed due to low confidence (2)

onnxruntime/python/tools/transformers/profile_result_processor.py:56

  • [nitpick] The error message could be more descriptive. Suggestion: 'Loading profile output from file: {profile_file}...'
print(f"loading profile output {profile_file} ...")

onnxruntime/python/tools/transformers/profile_result_processor.py:59

  • [nitpick] The variable name 'sess_time' is not very descriptive. Suggestion: 'session_times' or 'profile_data'.
sess_time = json.load(opened_file)

@github-actions github-actions Bot 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.

You can commit the suggested changes from lintrunner.

Comment thread onnxruntime/python/tools/transformers/profiler.py
Comment thread onnxruntime/python/tools/transformers/profiler.py
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.

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

Comment thread onnxruntime/python/tools/transformers/profile_result_processor.py Outdated
docstring should be the first lines
Adding Copyright notice
Prepend _ for private constants
Use a frozenset for quick lookup.

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

Comments suppressed due to low confidence (1)

onnxruntime/python/tools/transformers/profiler.py:16

  • The variable NODES_TYPE_CONTAINING_SUBGRAPH was removed from profiler.py and added to profile_result_processor.py. Ensure all references to this variable are updated accordingly.
NODES_TYPE_CONTAINING_SUBGRAPH = ["Scan", "Loop", "If"]

@jchen351
Jian Chen (jchen351) merged commit 655b3ef into main Jan 7, 2025
@jchen351
Jian Chen (jchen351) deleted the Cjian/profiler branch January 7, 2025 17:17
snnn (snnn) pushed a commit that referenced this pull request Jan 8, 2025
### Description
Separating result processor out from profiler.py without changing the
behaviors of current profile.py



### Motivation and Context
Less dependency and smaller code for processing profile from other
scenarios.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Tarek Ziade (tarekziade) pushed a commit to tarekziade/onnxruntime that referenced this pull request Jan 10, 2025
### Description
Separating result processor out from profiler.py without changing the
behaviors of current profile.py



### Motivation and Context
Less dependency and smaller code for processing profile from other
scenarios.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Guenther Schmuelling (guschmue) pushed a commit that referenced this pull request Jan 12, 2025
### Description
Separating result processor out from profiler.py without changing the
behaviors of current profile.py



### Motivation and Context
Less dependency and smaller code for processing profile from other
scenarios.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Ashrit Shetty (ashrit-ms) pushed a commit that referenced this pull request Mar 17, 2025
### Description
Separating result processor out from profiler.py without changing the
behaviors of current profile.py



### Motivation and Context
Less dependency and smaller code for processing profile from other
scenarios.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.

4 participants