RenderKit / oidn

Intel® Open Image Denoise library
https://www.openimagedenoise.org/
Apache License 2.0
1.78k stars 164 forks source link

oneDNN and oneMKL already, any plan for be compatible with new packages? #107

Closed jiapei100 closed 3 years ago

jiapei100 commented 3 years ago

mkl-dnn is not supported by Intel any more?

Cheers Pei

atafra commented 3 years ago

mkl-dnn and oneDNN are exactly the same library. oneDNN has been renamed multiple times, mkl-dnn is the original name. Open Image Denoise uses a slightly modified version of oneDNN, and currently there are no plans to switch to the main version. However, Open Image Denoise does not use oneMKL.

These are all internal implementation details and should not affect the usage or deployment of Open Image Denoise in any way.

jiapei100 commented 3 years ago

I've got onednn installed already.

➜  ~ apt show onednn
Package: onednn
Version: 2.1.3-6
Status: install ok installed
Priority: extra
Section: checkinstall
Maintainer: root@jiapei-Pan-Vision
Installed-Size: 53.2 MB
Provides: build
Download-Size: unknown
APT-Manual-Installed: yes
APT-Sources: /var/lib/dpkg/status
Description: Package created with checkinstall 1.6.3

oidn won't build with my installed onednn.

 Looking for TBB components tbb; ()
 Found TBB version 2021.2 at /usr/local
 CMake Error: File ....../oidn/mkl-dnn/include/dnnl_config.h.in does not exist.
 CMake Error at cmake/oidn_dnnl.cmake:13 (configure_file):
   configure_file Problem configuring file
 Call Stack (most recent call first):
   CMakeLists.txt:94 (include)

 CMake Error: File ....../oidn/mkl-dnn/include/dnnl_version.h.in does not exist.
 CMake Error at cmake/oidn_dnnl.cmake:17 (configure_file):
   configure_file Problem configuring file
 Call Stack (most recent call first):
   CMakeLists.txt:94 (include)

 Setting target version 1.3.0
 Configuring incomplete, errors occurred!
 See also "....../oidn/build/CMakeFiles/CMakeOutput.log".
 See also "....../oidn/build/CMakeFiles/CMakeError.log".
atafra commented 3 years ago

It's not possible to build Open Image Denoise with the regular oneDNN but you don't need oneDNN installed at all. The Open Image Denoise source code comes with its own version of oneDNN and normally you don't even need to know that oneDNN is used at all. The problem in your case seems to be that you did not clone the Git repository correctly. Open Image Denoise uses multiple submodules, and one of them is the internal oneDNN. To fix the issue, all you need to do is to clone the repository with all its submodules. Please read the Open Image Denoise compilation guide, which contains all the details: https://www.openimagedenoise.org/downloads.html

mfvescovi commented 3 years ago

Given the question was already discussed here, I'll simply follow up with more info instead of opening a new issue. Being a Debian Developer and main maintainer of Blender in Debian, I was about to package OIDN to add its capabilities to Debian Blender package but I had to face the fact that it's bundling a convenience copy of onednn that is quite different from the official one (already present in Debian archives), as you stated in the comment above. As per Debian Policy, it's discouraged the inclusion of such copies in the Debian archives, unless it's strictly necessary and possibly dropped in future. Now, my question: is there any possibility that those convenience copies will be dropped from the code in favor of official packages in the near future or should I not expect any change anyway? On the answer given, I'll decide if spending more spare time on packaging this library or not. Thanks in advance.

atafra commented 3 years ago

As I described before, this is not a convenience copy because it's different from the official oneDNN. This will not be dropped from OIDN and should not be replaced with anything else (e.g. the Debian package of oneDNN). OIDN should be built as it is to avoid incorrect behaviour and performance regressions. This should not cause any issues for Debian users, as this custom version of oneDNN is linked statically by OIDN.

khimaros commented 2 years ago

@atafra is there any chance for oneDNN to accept the patches you are carrying in OIDN? have you already spoken to them about it? or are you simply pinning to an unmodified known good version of oneDNN?

@mfvescovi in case they are carrying necessary patches, does Debian policy have an opt-out mechanism for cases where upstream is intentionally carrying a delta against one of their dependencies?

atafra commented 2 years ago

We won't be able to switch to an unmodified version of oneDNN in the foreseeable future.