Bug 2251785 - Review Request: openvino - Open Visual Inference And Optimization toolkit for AI inference
Summary: Review Request: openvino - Open Visual Inference And Optimization toolkit for...
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Tom Rix
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/openvinotoolkit/op...
Whiteboard:
Depends On: 2254599
Blocks: ML-SIG
TreeView+ depends on / blocked
 
Reported: 2023-11-27 15:18 UTC by Ali Erdinc Koroglu
Modified: 2024-05-03 11:39 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
trix: fedora-review?


Attachments (Terms of Use)
The .spec file difference from Copr build 6766923 to 7400792 (8.44 KB, patch)
2024-05-03 11:39 UTC, Fedora Review Service
no flags Details | Diff

Description Ali Erdinc Koroglu 2023-11-27 15:18:48 UTC
SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-aarch64/06698631-openvino/openvino.spec
SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-aarch64/06698631-openvino/openvino-2023.2.0-1.fc40.src.rpm

Description:
OpenVINO is an open-source toolkit for optimizing and deploying AI inference.
It can be used to develop applications and solutions based on deep learning
tasks, such as: emulation of human vision, automatic speech recognition, 
natural language processing, recommendation systems, etc.

Reproducible: Always

Comment 1 Fedora Review Service 2023-11-27 21:02:48 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6700410
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/srpm-builds/06700410/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Tom Rix 2023-12-01 17:15:24 UTC
On first look, the big problem is the third_party/  dir in the tarball.
These should should be their own packages.
If they are not needed, then the third_party dir should be rm-ed in %prep

Comment 3 Tom Rix 2023-12-14 18:42:00 UTC
What I mean is these third party packages need to be their own packages.
Please see 
https://bugzilla.redhat.com/show_bug.cgi?id=2254599
this removes the openvino/thirdparty/json dependency.
And if you have the time, please review.

Comment 4 Ali Erdinc Koroglu 2023-12-14 19:12:30 UTC
Hello Tom, I disabled them in the patch file, but do you want me to delete them instead?

--- a/CMakeLists.txt	2023-11-22 08:22:33.730665512 +0200
+++ b/CMakeLists.txt	2023-11-24 00:34:16.675363131 +0200
@@ -38,7 +38,7 @@
 include(cmake/features.cmake)
 
 # These options are shared with 3rdparty plugins by means of developer package
-include(cmake/dependencies.cmake)
+# include(cmake/dependencies.cmake)
 
 if(ENABLE_COVERAGE)
     include(cmake/coverage.cmake)
@@ -133,7 +133,7 @@
     include(cmake/test_model_zoo.cmake)
 endif()
 
-include(thirdparty/dependencies.cmake)
+# include(thirdparty/dependencies.cmake)
 add_subdirectory(src)
 
 if(ENABLE_SAMPLES OR ENABLE_TESTS)
@@ -146,10 +146,10 @@
 endif()
 
 include(cmake/extra_modules.cmake)
-add_subdirectory(docs)
-add_subdirectory(tools)
-add_subdirectory(scripts)
-add_subdirectory(licensing)
+# add_subdirectory(docs)
+# add_subdirectory(tools)
+# add_subdirectory(scripts)
+# add_subdirectory(licensing)
 
 if(ENABLE_TESTS)
     # layers and other more high-level / e2e tests
--- a/src/CMakeLists.txt	2023-11-22 08:23:48.722438766 +0200
+++ b/src/CMakeLists.txt	2023-11-22 08:25:27.304120882 +0200
@@ -8,7 +8,9 @@
     ov_add_compiler_flags(-Wmissing-declarations)
 endif()
 
-include(cmake/install_tbb.cmake)
+# dont install_tbb
+# include(cmake/install_tbb.cmake)
+include(cmake/ov_parallel.cmake)
 
 # CC library should be registered before other cc targets
 add_subdirectory(common)
--- a/src/plugins/intel_cpu/thirdparty/CMakeLists.txt	2023-11-22 08:35:23.572344257 +0200
+++ b/src/plugins/intel_cpu/thirdparty/CMakeLists.txt	2023-11-22 08:35:40.634293194 +0200
@@ -115,7 +115,7 @@
         list(APPEND CMAKE_MODULE_PATH "${intel_cpu_thirdparty_SOURCE_DIR}")
     endif()
 
-    add_subdirectory(onednn EXCLUDE_FROM_ALL)
+    # add_subdirectory(onednn EXCLUDE_FROM_ALL)
 
     # install static libraries
     ov_install_static_lib(dnnl ${OV_CPACK_COMP_CORE})

Comment 5 Tom Rix 2023-12-15 01:12:04 UTC
Unwinding third_party can be complicated.
To prove you do not needed it, remove it in prep
Like
https://src.fedoraproject.org/rpms/python-torch/blob/rawhide/f/python-torch.spec#_178

I was really only looking at the toplevel thirdparty/ , but it looks like from above that they are also in the plugins.

Comment 7 Fedora Review Service 2023-12-18 15:12:49 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6766923
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/06766923-openvino/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 8 Tom Rix 2024-03-21 13:11:13 UTC
At least the licensecheck seems to be missing from the above auto review.
Spec looks generally fine.

This line should not be needed.
        -DCMAKE_CXX_FLAGS="%{optflags} -Wformat -Wformat-security" \ 

There is also at least third party for 
src/bindings/python/thirdparty
src/plugins/intel_gpu/thirdparty

That needs to be removed.

I think the two small plugin subpackages should be rolled into the the main package.

There should be a way, even a manual way to test.

The samples/ would be good to include into the devel package.

Can python be enabled ? python is widely used in AI so this would be good.

Comment 9 Ali Erdinc Koroglu 2024-05-03 08:36:38 UTC
Hello Tom, it's been a while sorry about it.
Python API, PyTorch + IR frontend supports are enabled and tested

SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07399413-openvino/openvino.spec
SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/07399413-openvino/openvino-2024.0.0-1.fc41.src.rpm

Known issues
- onednn (opevino forked version) and intel-mlas bundled to compile intel_cpu_plugin 
- onnx disabled (requested version of Google Protobuf is 3.20.3)
- aarch64 disabled (https://github.com/ARM-software/ComputeLibrary not packed yet)
- 2024.1.0 version has some more thirdparty deps.

PS: I added CXX_FLAGS="%{optflags} -Wformat -Wformat-security" to  fix src/bindings/c/src/ov_dimension.cpp compilation error
cc1plus: error: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Werror=format-security]

Manual test results:
[aekoroglu@test samples]$ python sync_benchmark.py /home/aekoroglu/models/public/googlenet-v1/FP32/googlenet-v1.xml
[ INFO ] OpenVINO:
[ INFO ] Build ................................. 2024.0.0-000--
[ INFO ] Count:          1856 iterations
[ INFO ] Duration:       10000.79 ms
[ INFO ] Latency:
[ INFO ]     Median:     4.91 ms
[ INFO ]     Average:    5.39 ms
[ INFO ]     Min:        4.46 ms
[ INFO ]     Max:        15.30 ms
[ INFO ] Throughput: 185.59 FPS
[aekoroglu@test samples]$ python throughput_benchmark.py /home/aekoroglu/models/public/googlenet-v1/FP32/googlenet-v1.xml
[ INFO ] OpenVINO:
[ INFO ] Build ................................. 2024.0.0-000--
[ INFO ] Count:          2893 iterations
[ INFO ] Duration:       10017.42 ms
[ INFO ] Latency:
[ INFO ]     Median:     13.50 ms
[ INFO ]     Average:    13.71 ms
[ INFO ]     Min:        11.57 ms
[ INFO ]     Max:        28.85 ms
[ INFO ] Throughput: 288.80 FPS

https://github.com/openvinotoolkit/openvino/tree/2024.0.0/samples/python/benchmark/sync_benchmark
https://github.com/openvinotoolkit/openvino/tree/2024.0.0/samples/python/benchmark/throughput_benchmark

Comment 10 Fedora Review Service 2024-05-03 11:39:42 UTC
Created attachment 2031038 [details]
The .spec file difference from Copr build 6766923 to 7400792

Comment 11 Fedora Review Service 2024-05-03 11:39:44 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7400792
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251785-openvino/fedora-rawhide-x86_64/07400792-openvino/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.


Note You need to log in before you can comment on or make changes to this bug.