openvinotoolkit / openvino

OpenVINO™ is an open-source toolkit for optimizing and deploying AI inference
https://docs.openvino.ai
Apache License 2.0
7.25k stars 2.26k forks source link

[Good First Issue]: Compile OpenVINO on macOS with Xcode cmake generator #19891

Open ilya-lavrenov opened 1 year ago

ilya-lavrenov commented 1 year ago

Context

OpenVINO is a cross platform product, which can be built with different compilers, their versions. But different compilers and cmake generators can slightly adjust their option, so the product compilation emits warnings.

The proper way is to align compiler options, so the product compilation become less OS / compiler / cmake generator dependent. E.g. if MSVC on Windows often emits signed / unsigned types conversion, we need to find similar option for Clang / GCC and enable by default and fix warnings.

With this task, you can understand simple OpenVINO data structures, get an experience with working with compiler flags, making a portable code base.

What needs to be done?

Run cmake with:

cmake -G Xcode -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -S <openvino source> -B <build dir>

And build with:

cmake --build . --parallel --config Release -- -IDEBuildingContinueBuildingAfterErrors=YES

And fix all compilation errors, adjust cmake flags where it's needed.

Example Pull Requests

https://github.com/openvinotoolkit/openvino/pull/19828

Resources

Contact points

@ilya-lavrenov

pedrolinhares commented 1 year ago

I would like to work on this issue.

pedrolinhares commented 1 year ago

@ilya-lavrenov I think I fixed most of the compile error but getting a link error now, it can't find libopenvino.2023.2.0.dylib in my Release folder. I think there is issue when building the shared library, if I look at my Release folder I see dylibs:

image

So, is it supposed to be like this? It looks like libopenvino.2320.dylib is supposed to be libopenvino.2023.2.0.dylib. What am I missing? Any idea why the shared library gets the wrong name?

ilya-lavrenov commented 1 year ago

Hi @pedrolinhares Here we define a library version: https://github.com/openvinotoolkit/openvino/blob/affceaa32bdc98a75e848b28f49095eaaa2e0b39/cmake/developer_package/version.cmake#L228-L238 Which in fact creates:

libopenvino.dylib => symlink to libopenvino.2320.dylib
libopenvino.2320.dylib => symlink to libopenvino.2023.2.0.dylib
libopenvino.2023.2.0.dylib

Could you please send as a PR your intermediate changes?

Some information about this property here https://cmake.org/cmake/help/latest/prop_tgt/VERSION.html#mach-o-versions. Maybe you need to try to use macOS specific properties described there

pedrolinhares commented 1 year ago

Hi @ilya-lavrenov, here are my intermediate changes: #20409. Let me know what you think. As I mentioned, this is not successfully building yet due to linking errors. It's not generating the expected libraries such as libopenvino.2023.2.0.dylib.

pedrolinhares commented 1 year ago

hi @ilya-lavrenov, the OpenVINO project is successfully building now for apple silicon and Xcode 15. Would you mind reviewing the PR one more time, please. Also, I have a PR for the oneDNN repository too that needs to be checked in for OpenVINO to build. oneDNN PR

For some libraries depending on Object libraries I had to add a dummy cpp file to make Xcode build the library, otherwise it won't build if it depends only on objects. Let me know what you think is the best way to tackle this.

p-wysocki commented 11 months ago

@pedrolinhares @ilya-lavrenov is this task still relevant? I see that https://github.com/openvinotoolkit/openvino/pull/20409 has hasn't been maintained for a while.

p-wysocki commented 11 months ago

@ilya-lavrenov I'm a bit lost here, should we reopen this issue for other contributors?

p-wysocki commented 10 months ago

I guess so. :)

Also, I am happy to announce that we have created a channel dedicated to Good First Issues support on our Intel DevHub Discord server! Join it to receive support, engage in discussions, ask questions and talk to OpenVINO developers.

BryanZaneee commented 9 months ago

Hello, I would like to work on this issue!

mlukasze commented 9 months ago

hey @BryanZaneee - so not it's yours :) Good luck and have fun! :)

p-wysocki commented 8 months ago

Hello @BryanZaneee, are you still working on that issue? Is there anything we can help you with?

BryanZaneee commented 8 months ago

Hey my apologies I swore I had pushed the changes. I submitted a pull request now to address some of the compiler errors, sorry again for the delay!

p-wysocki commented 8 months ago

No worries! Thank you for your contribution. Just linking the PR for reference: https://github.com/openvinotoolkit/openvino/pull/22932

inbasperu commented 7 months ago

Hello, I would like to work on this issue!

github-actions[bot] commented 7 months ago

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

inbasperu commented 7 months ago

.take

github-actions[bot] commented 7 months ago

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

inbasperu commented 7 months ago

Hello @ilya-lavrenov, I've started working on this task.

System Information:

I built the project without the DCMAKE_COMPILE_WARNING_AS_ERROR=ON flag to ensure that the build is successful without considering compiler warnings.

Next, I enabled the compile warning flag and encountered 365 failures in total, most of which were due to implicit type conversions.

To address this issue, I considered different approaches:

Approach 1: Modify the Source Code. If I am allowed to change the source code to fix the issue, we can perform a static cast of the data type. I feel this is a better approach than suppressing all the warnings.

Approach 2: Conditionally Apply the Flag for Apple ARM64 Builds. Make changes to the CMakeLists.txt where the samples' executable target is defined:

# Detect if we are on an Apple ARM64 system
if(APPLE AND CMAKE_SYSTEM_PROCESSOR MATCHES "arm64")
    # Add the compiler flag to ignore the integer conversion warnings
    add_compile_options(-Wno-shorten-64-to-32)
endif()

Modify build_samples.sh:

# Determine if we are on Apple ARM64
if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "arm64" ]]; then
    # Add the compiler flag to the build command
    CXXFLAGS+=" -Wno-error=shorten-64-to-32"
fi

Pass the modified compiler flag:

$CMAKE_EXEC -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS="$CXXFLAGS" "$SAMPLES_SOURCE_DIR"

Before I make any changes, I thought it would be best to discuss things first. Please let me know how I should proceed.

inbasperu commented 6 months ago

Hi, could someone please check on this for me? Thanks!

p-wysocki commented 6 months ago

cc @ilya-lavrenov