Skip to content

[ROCm] Add AveragePool, GlobalAveragePool, MaxPool, GlobalMaxPool Ops#11968

Merged
ytaous merged 5 commits intomicrosoft:masterfrom
ROCm:xinyazhang/pool_v2
Aug 3, 2022
Merged

[ROCm] Add AveragePool, GlobalAveragePool, MaxPool, GlobalMaxPool Ops#11968
ytaous merged 5 commits intomicrosoft:masterfrom
ROCm:xinyazhang/pool_v2

Conversation

@xinyazhang
Copy link
Contributor

@xinyazhang xinyazhang commented Jun 23, 2022

Description: [ROCm] Add AveragePool, GlobalAveragePool, MaxPool, GlobalMaxPool Ops.

Motivation and Context

  • Why is this change required? What problem does it solve?
    • AveragePool, GlobalAveragePool, MaxPool, GlobalMaxPool Ops are missing in ROCm provider.

@ytaous
Copy link
Contributor

ytaous commented Jul 15, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Jul 15, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 15, 2022

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 15, 2022

Azure Pipelines successfully started running 8 pipeline(s).

@ytaous
Copy link
Contributor

ytaous commented Jul 18, 2022

can u pls fix lint error?

@xinyazhang
Copy link
Contributor Author

xinyazhang commented Jul 18, 2022

can u pls fix lint error?

I just want to confirm we do really need fix the following lint error

-      // BuildKernelCreateInfo<ONNX_OPERATOR_VERSIONED_TYPED_KERNEL_CLASS_NAME(kRocmExecutionProvider, kOnnxDomain, 7, 9, float, AveragePool)>,
+      BuildKernelCreateInfo<ONNX_OPERATOR_VERSIONED_TYPED_KERNEL_CLASS_NAME(kRocmExecutionProvider, kOnnxDomain, 7, 9, float, AveragePool)>,

To me it's pretty clearly a false alert, unless we do want to reformat the whole rocm_execution_provider.cc file

@ytaous
Copy link
Contributor

ytaous commented Jul 18, 2022

it's complaining about length > 120, pls split that into 2 lines. thx

@ytaous
Copy link
Contributor

ytaous commented Jul 18, 2022 •
edited
Loading

it's complaining about length > 120, pls split that into 2 lines. thx

https://github.com/microsoft/onnxruntime/pull/11968/files#diff-a2b83b088670cbcef15a6e77d6533f559003ed42894467bde9964d157cc033be

@xinyazhang xinyazhang force-pushed the xinyazhang/pool_v2 branch from 1305bcc to 5eb33cd Compare July 18, 2022 20:17
@ytaous ytaous self-requested a review July 18, 2022 21:25
@ytaous
Copy link
Contributor

ytaous commented Jul 18, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Jul 18, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 18, 2022

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 18, 2022

Azure Pipelines successfully started running 8 pipeline(s).

ytaous
ytaous reviewed Jul 19, 2022
View reviewed changes
Lafi7e
Lafi7e reviewed Jul 21, 2022
View reviewed changes
@ytaous
Copy link
Contributor

ytaous commented Jul 26, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Jul 26, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 26, 2022

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Jul 26, 2022

Azure Pipelines successfully started running 9 pipeline(s).

@ytaous
Copy link
Contributor

ytaous commented Jul 27, 2022

The CI - orttraining-linux-gpu-ci-pipeline should have been fixed, if it's still failing, pls sync from master again, I'll trigger the build, thx

@xinyazhang
Copy link
Contributor Author

xinyazhang commented Jul 28, 2022

The CI - orttraining-linux-gpu-ci-pipeline should have been fixed, if it's still failing, pls sync from master again, I'll trigger the build, thx

I saw ' orttraining-linux-gpu-ci-pipeline Successful in 95m — Build #20220725.34 succeeded '

Lafi7e
Lafi7e reviewed Aug 2, 2022
View reviewed changes
@xinyazhang xinyazhang mentioned this pull request Aug 2, 2022
Merged
@xinyazhang
Copy link
Contributor Author

xinyazhang commented Aug 2, 2022 

$ (cd ./branch_build/$(git branch --show-current)/RelWithDebInfo/; ./onnxruntime_test_all --gtest_filter='QLinearPoolTest.*:PoolTest.*:QLinearGlobalAveragePool.*:')
...
[----------] Global test environment tear-down
[==========] 100 tests from 3 test suites ran. (3855 ms total)
[  PASSED  ] 100 tests.

GPU Utilization confirmed with AMD_LOG_LEVEL=3

Full logs:
pool_v2.log
pool_v2_gpu.log

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 9 pipeline(s).

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 8 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 9 pipeline(s).

Lafi7e
Lafi7e previously approved these changes Aug 3, 2022
View reviewed changes
ytaous
ytaous previously approved these changes Aug 3, 2022
View reviewed changes
@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022 •
edited
Loading

please resolve the conflicts, thx

@xinyazhang xinyazhang dismissed stale reviews from ytaous and Lafi7e via f24fccc August 3, 2022 18:03
@xinyazhang
Copy link
Contributor Author

xinyazhang commented Aug 3, 2022 •
edited
Loading

please resolve the conflicts, thx

Conflicts resolved.

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, onnxruntime-python-checks-ci-pipeline

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 3, 2022

Azure Pipelines successfully started running 8 pipeline(s).

@ytaous ytaous merged commit 77cab7a into microsoft:master Aug 3, 2022
@ytaous
Copy link
Contributor

ytaous commented Aug 3, 2022

Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lafi7e Lafi7e Lafi7e left review comments

@weixingzhang weixingzhang Awaiting requested review from weixingzhang

@mindest mindest Awaiting requested review from mindest

+1 more reviewer

@ytaous ytaous ytaous 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.

3 participants

@xinyazhang @ytaous @Lafi7e