Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OpenVINO-EP] V3.4 Release with OpenVINO 2021.4.2 LTS Release #9848

Merged
merged 6 commits into from
Nov 23, 2021

Conversation

MaajidKhan
Copy link
Contributor

Description: [OpenVINO-EP] V3.4 Release

Motivation and Context:

  • Making OpenVINO EP compatible with the latest OpenVINO 2021.4.2 Release
  • Added windows fix (windows build was broken)
  • Added c# Nuget package build fix (c# build was broken)
  • Modified Hetero feature logic in the getcapability()

@oliviajain
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline

@oliviajain
Copy link
Contributor

/azp run MacOS NoContribops CI Pipeline, 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

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oliviajain
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@oliviajain oliviajain closed this Nov 23, 2021
@oliviajain oliviajain reopened this Nov 23, 2021
@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@oliviajain oliviajain self-requested a review November 23, 2021 16:34
@oliviajain
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@oliviajain
Copy link
Contributor

/azp run MacOS NoContribops CI Pipeline, 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

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@oliviajain
Copy link
Contributor

/azp run onnxruntime-python-checks-ci-pipeline, Linux GPU CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

# Download and build ONNX Runtime
cd ${MY_ROOT} && \
git clone --recursive -b ${ONNXRUNTIME_BRANCH} ${ONNXRUNTIME_REPO} && \
/bin/sh onnxruntime/dockerfiles/scripts/install_common_deps.sh && \
pip install onnx==1.9 && \
cd ${MY_ROOT}/onnxruntime && ./build.sh --config Release --update --build --parallel --use_openvino ${DEVICE} --build_nuget --build_shared_lib && \
cp ${MY_ROOT}/onnxruntime/build/Linux/Release/Microsoft.ML.OnnxRuntime.Managed* ${MY_ROOT}/onnxruntime/build/Linux/Release/nuget-artifacts && \
Copy link
Contributor

@oliviajain oliviajain Nov 23, 2021

Choose a reason for hiding this comment

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

Out of curiosity, why wasn't this required before? Is this because of Scott's change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oliviajain Earlier there used to be two nuget packages that used to get generated in nuget-artifacts folder, but due to recent changes in nuget build, now only OpenVINO nuget is being generated in nuget-artifacts folder, so we are copying the other one also to the artifacts folder.

saharfraza and others added 6 commits November 24, 2021 01:33
*Modified Hetero Feature logic. In Hetero,
if the operator to be marked true in getcapability(),
it should be supported by either of the devices
specified with HETERO in the device_type.

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>
*Copying Managed nuget to nugets artifacts
directory

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>
@oliviajain
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oliviajain oliviajain merged commit 0ae0f29 into microsoft:master Nov 23, 2021
@@ -960,7 +960,11 @@ if (WIN32)
# issued by thrust nonstandard extension used: nameless struct/union
list(APPEND ORT_WARNING_FLAGS "/wd4201")
# warning C4800: Implicit conversion from 'X' to bool. Possible information loss
list(APPEND ORT_WARNING_FLAGS "/w34800")
if (onnxruntime_USE_OPENVINO)
Copy link
Member

Choose a reason for hiding this comment

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

where does this warning get emitted? should this have only been added for openvino targets?
i.e. follow this pattern as example:

set(DISABLED_WARNINGS_FOR_TVM "/wd4100" "/wd4244" "/wd4251" "/wd4267" "/wd4275" "/wd4389" "/wd4456")

and
target_compile_options(onnxruntime_providers_nuphar PRIVATE ${DISABLED_WARNINGS_FOR_TVM})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jywu-msft Hi George. I think, very recently this level 3 warning flag has been added by default /w34800 for WIN32 builds. so, we had to put a check here, to disable it for OpenVINO EP.

we did even try disabling this warning (/wd4800) at a later stage in the cmake and it wasn't effective and not working.

I did even try the way as commented above by you where I set the warnings to be disabled in cmake and called it out in onnxruntime_providers.cmake for OpenVINO EP and it didn't work either.

Copy link
Member

Choose a reason for hiding this comment

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

ok. thanks for checking. i'll take a look at it later as well.

jingyanwangms pushed a commit that referenced this pull request Nov 30, 2021
* Changes to ensure openvino build go through in Windows

* Modified Hetero plugin Logic

*Modified Hetero Feature logic. In Hetero,
if the operator to be marked true in getcapability(),
it should be supported by either of the devices
specified with HETERO in the device_type.

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>

* OV updated to 2021.4.2 version

* OV updated to 2021.4.2 version

* Updated OV to 2021.4.2 version, mono download  link and dotnet version

* Copying Managed nugets in openvino c# docker file

*Copying Managed nuget to nugets artifacts
directory

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>

Co-authored-by: saharfraza <sfatima.3001@gmail.com>
Co-authored-by: mayavijx <mayax.vijayan@intel.com>
Co-authored-by: Aravind Gunda <aravindx.gunda@intel.com>
(cherry picked from commit 0ae0f29)
jingyanwangms added a commit that referenced this pull request Nov 30, 2021
* Fix memset size (#9840)

(cherry picked from commit d012d9f)

* [js/web] do not use nodejs type 'Buffer' in web (#9839)

* [js/web] do not use nodejs type 'Buffer' in web

* resolve comments and validate tests

* remove 'Buffer' in test

(cherry picked from commit a3ebc5e)

* Fix potential data race with OrtValue usage in Python (#9841)

(cherry picked from commit 18fd2cf)

* [OpenVINO-EP] V3.4 Release with OpenVINO 2021.4.2 LTS Release (#9848)

* Changes to ensure openvino build go through in Windows

* Modified Hetero plugin Logic

*Modified Hetero Feature logic. In Hetero,
if the operator to be marked true in getcapability(),
it should be supported by either of the devices
specified with HETERO in the device_type.

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>

* OV updated to 2021.4.2 version

* OV updated to 2021.4.2 version

* Updated OV to 2021.4.2 version, mono download  link and dotnet version

* Copying Managed nugets in openvino c# docker file

*Copying Managed nuget to nugets artifacts
directory

Signed-off-by: MaajidKhan <n.maajidkhan@gmail.com>

Co-authored-by: saharfraza <sfatima.3001@gmail.com>
Co-authored-by: mayavijx <mayax.vijayan@intel.com>
Co-authored-by: Aravind Gunda <aravindx.gunda@intel.com>
(cherry picked from commit 0ae0f29)

* no fallback when enforcing explicit EP registration. (#9863)

* no fallback when enforcing explicit EP registration.

* add explicit ep registrations for python.

(cherry picked from commit 1e9e57d)

* layernorm throw error if input has no data (#9837)

(cherry picked from commit bf716e6)

* [js/node] npm audit fix (#9861)

(cherry picked from commit 27e337e)

* [python manylinux package] emit warning if missing CUDA/TensorRT dependency causes ld_preload to fail and user tries to register either CUDA/TensorRT EP (#9872)

* add warning if ld_preload fails for CUDA or TRT when trying to register either provider

* refactor

* change wording from register to create

(cherry picked from commit ec9b0ed)

* QDQ tool modification part2 (#9720)

* Add finetuned qdq options

* Add description

* Add unit tests

* Modify for channel axis

* Remove too specific feature. Move this implementation to e2e example

* Add OpTypesSupportPerChannelQuantization

* fix bug for unit test

* Keep flags OpTypesSupportPerChannelQuantization and QDQChannelAxis for internal use

Will have a follow-up PR to fine tune the code

* remove unnecessary warning

Co-authored-by: stevenlix <38092805+stevenlix@users.noreply.github.com>
Co-authored-by: Yufeng Li <liyufeng1987@gmail.com>
(cherry picked from commit 0baf687)

* Cancel transpose optimizer for resize (#9870)

* cancel transpose optimizer for resize

* add UT

* addressing comments

* fix build err

(cherry picked from commit 16bfd3c)

* Add build option to enable cuda profiling (#9875)

(cherry picked from commit 9345894)

Co-authored-by: Dmitri Smirnov <yuslepukhin@users.noreply.github.com>
Co-authored-by: Yulong Wang <7679871+fs-eire@users.noreply.github.com>
Co-authored-by: Hariharan Seshadri <shariharan91@gmail.com>
Co-authored-by: Maajid khan <n.maajidkhan@gmail.com>
Co-authored-by: George Wu <jywu@microsoft.com>
Co-authored-by: Ye Wang <52801275+wangyems@users.noreply.github.com>
Co-authored-by: Chi Lo <54722500+chilo-ms@users.noreply.github.com>
Co-authored-by: RandySheriffH <48490400+RandySheriffH@users.noreply.github.com>
@ricklina90
Copy link

Hi, According this article #9271.
I tried to build OpenVINO-EP for C# nuget package again in Windows environment, but build still failed.
Anyone as an idea of possible solution? Thanks a lot!

error

@MaajidKhan
Copy link
Contributor Author

@oliviajain @jywu-msft I see the release tag 1.10 is removed to this PR?
will it be taken care of while cherry picking the commits for ONNXRT v1.10 release?

@oliviajain
Copy link
Contributor

@oliviajain @jywu-msft I see the release tag 1.10 is removed to this PR? will it be taken care of while cherry picking the commits for ONNXRT v1.10 release?

The PR has already been cherry picked in the release branch. You can see it in the list of commits here.

@sfatimar sfatimar deleted the openvino-ep-2021.4.2-v3.4 branch September 14, 2022 07:04
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.

9 participants