Skip to content

Modify MIGraphX EP for Accuracy tests#13455

Merged
cloudhan merged 13 commits into
microsoft:mainfrom
TedThemistokleous:modify_ep_for_tests
Nov 27, 2022
Merged

Modify MIGraphX EP for Accuracy tests#13455
cloudhan merged 13 commits into
microsoft:mainfrom
TedThemistokleous:modify_ep_for_tests

Conversation

@TedThemistokleous
Copy link
Copy Markdown
Contributor

@TedThemistokleous TedThemistokleous commented Oct 26, 2022 •
edited
Loading

Description

Changes to allow MIGraphX EP while leveraging the work ROCm has done for the EP for getting additional accuracy tests running.

Motivation and Context

Allows MIGraphX EP to run the following additional tests

test_parity_gelu.py
test_parity_layernorm.py
test_parity_huggingface_gpt_attention.py
convert_to_onnx.py

Reference to the Rocm EP changes: #13306

Also adds support to get MIGraphX to run eval_squad.py

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Oct 26, 2022

This pull request fixes 1 alert when merging f0ea9d1 into 70b73af - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable

@TedThemistokleous TedThemistokleous marked this pull request as ready for review November 1, 2022 19:07
tianleiwu
tianleiwu previously approved these changes Nov 4, 2022
View reviewed changes
@TedThemistokleous TedThemistokleous force-pushed the modify_ep_for_tests branch 3 times, most recently from b546d4c to 7ca3601 Compare November 4, 2022 22:14
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Nov 5, 2022

This pull request fixes 1 alert when merging 7ca3601 into ab9ac2a - view on LGTM.com

fixed alerts:

  • 1 for Unused local variable

@TedThemistokleous
Copy link
Copy Markdown
Contributor Author

TedThemistokleous commented Nov 8, 2022

@tianleiwu is there anything else we need for this PR to go through?

@TedThemistokleous TedThemistokleous mentioned this pull request Nov 15, 2022
Merged
@microsoft microsoft deleted a comment from lgtm-com Bot Nov 21, 2022
@microsoft microsoft deleted a comment from lgtm-com Bot Nov 21, 2022
@cloudhan cloudhan self-requested a review November 21, 2022 11:54
@cloudhan

This comment was marked as resolved.

Sign in to view
@cloudhan

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@microsoft microsoft deleted a comment from lgtm-com Bot Nov 21, 2022
@microsoft microsoft deleted a comment from lgtm-com Bot Nov 21, 2022
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/models/bert/eval_squad.py Outdated
)

result["provider"] = "CUDAExecutionProvider" if args.use_gpu else "CPUExecutionProvider"
result["provider"] = args.provider if use_gpu else "CPUExecutionProvider"
Copy link
Copy Markdown
Contributor

@cloudhan cloudhan Nov 21, 2022

Choose a reason for hiding this comment

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

Why it cannot be result["provider"] = args.provider

cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/models/bert/eval_squad.py Outdated
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/test/python/transformers/parity_utilities.py
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/benchmark_helper.py Outdated
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Outdated
cloudhan
cloudhan reviewed Nov 21, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Outdated
github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 23, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Fixed
cloudhan
cloudhan reviewed Nov 24, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/benchmark_helper.py Outdated
assert (
"CUDAExecutionProvider" in onnxruntime.get_available_providers()
), "Please install onnxruntime-gpu package to test GPU inference."
assert any(
Copy link
Copy Markdown
Contributor

@cloudhan cloudhan Nov 24, 2022

Choose a reason for hiding this comment

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

You forgot to remove the any() here. isdisjoint have already returned a bool here.

Copy link
Copy Markdown
Contributor Author

@TedThemistokleous TedThemistokleous Nov 24, 2022

Choose a reason for hiding this comment

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

Ah right. Let me fix this

github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 24, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Fixed
github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 24, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Fixed
@TedThemistokleous TedThemistokleous force-pushed the modify_ep_for_tests branch 3 times, most recently from 180e5ca to 8301ffc Compare November 24, 2022 22:50
github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 24, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py Fixed
github-advanced-security[bot]
github-advanced-security AI found potential problems Nov 24, 2022
View reviewed changes
Comment thread onnxruntime/python/tools/transformers/optimizer.py
gpu_ep.append("MIGraphXExecutionProvider")
gpu_ep.append("ROCMExecutionProvider")

session = onnxruntime.InferenceSession(onnx_model_path, sess_options, providers=gpu_ep, **kwargs)

Check notice

Code scanning / CodeQL

Unused local variable

Variable session is not used.
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Nov 25, 2022

This pull request introduces 1 alert when merging 7f03036 into 4ca62b9 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog.

@cloudhan

This comment was marked as resolved.

Sign in to view
@cloudhan

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@azure-pipelines

This comment was marked as resolved.

Sign in to view
@cloudhan cloudhan merged commit c6bea4f into microsoft:main Nov 27, 2022
@mindest mindest mentioned this pull request Dec 2, 2022
Merged
@groenenboomj groenenboomj mentioned this pull request Dec 2, 2022
Closed
mindest added a commit that referenced this pull request Dec 3, 2022
@mindest
fix a wrong assert condition in benchmark_helper (#13821)
f34ebbc 
### Description
fix a wrong assert condition in benchmark_helper.py (introduced in
#13455)
groenenboomj added a commit to ROCm/onnxruntime that referenced this pull request Dec 8, 2022
@TedThemistokleous @groenenboomj
Modify MIGraphX EP for Accuracy tests (microsoft#13455)
f073701 
Allows MIGraphX EP to run the following additional tests. Also adds support to get MIGraphX to run eval_squad.py

Reference to the Rocm EP changes: microsoft#13306

Co-authored-by: Joseph Groenenboom <joseph.groenenboom@amd.com>
Co-authored-by: Ted Themistokleous <tthemist@amd.com>
@TedThemistokleous TedThemistokleous mentioned this pull request Dec 12, 2022
Merged
baijumeswani pushed a commit that referenced this pull request Dec 13, 2022
@mindest @baijumeswani
fix a wrong assert condition in benchmark_helper (#13821)
e407b18 
### Description
fix a wrong assert condition in benchmark_helper.py (introduced in
#13455)
simon-moo pushed a commit to simon-moo/onnxruntime that referenced this pull request Dec 26, 2022
@TedThemistokleous @groenenboomj @fuhengwu2021
Modify MIGraphX EP for Accuracy tests (microsoft#13455)
09bc4e1 
Allows MIGraphX EP to run the following additional tests. Also adds support to get MIGraphX to run eval_squad.py

Reference to the Rocm EP changes: microsoft#13306

Co-authored-by: Joseph Groenenboom <joseph.groenenboom@amd.com>
Co-authored-by: Ted Themistokleous <tthemist@amd.com>
simon-moo pushed a commit to simon-moo/onnxruntime that referenced this pull request Dec 26, 2022
@mindest @fuhengwu2021
fix a wrong assert condition in benchmark_helper (microsoft#13821)
8cd9a7b 
### Description
fix a wrong assert condition in benchmark_helper.py (introduced in
microsoft#13455)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tianleiwu tianleiwu tianleiwu left review comments

+1 more reviewer

@cloudhan cloudhan cloudhan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TedThemistokleous @cloudhan @tianleiwu @github-advanced-security @groenenboomj