Skip to content

Conversation

@jchen351
Copy link
Contributor

@jchen351 jchen351 commented Jan 4, 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.

@jchen351 jchen351 requested a review from justinchuby January 4, 2025 18:40
Copy link
Contributor

@github-actions github-actions bot left a comment

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.

justinchuby
justinchuby previously approved these changes Jan 6, 2025
@justinchuby justinchuby requested a review from Copilot January 6, 2025 07:10
Copy link
Contributor

Copilot AI left a comment

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.

Copy link
Contributor

@github-actions github-actions bot left a comment

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.

@jchen351 jchen351 requested a review from Copilot January 6, 2025 20:09
Copy link
Contributor

Copilot AI left a comment

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)

Copy link
Contributor

@github-actions github-actions bot left a comment

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.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@jchen351 jchen351 requested a review from Copilot January 6, 2025 20:16
Copy link
Contributor

Copilot AI left a comment

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.

docstring should be the first lines
Adding Copyright notice
Prepend _ for private constants
Use a frozenset for quick lookup.
@jchen351 jchen351 requested a review from Copilot January 6, 2025 20:40
Copy link
Contributor

Copilot AI left a comment

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 jchen351 merged commit 655b3ef into main Jan 7, 2025
94 of 96 checks passed
@jchen351 jchen351 deleted the Cjian/profiler branch January 7, 2025 17:17
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>
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>
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-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.

3 participants