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

ENH: Add ITKVkFFTBackend remote module #3363

Merged

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Apr 6, 2022

Changes:

  • Adds https://github.com/InsightSoftwareConsortium/ITKVkFFTBackend as a remote module
  • Removes deprecated ITK/FindOpenCL.cmake in favor of CMake's built-in procedure for finding OpenCL, which extends the list of variables defined for referencing OpenCL and is compatible with CMake>=3.10.
  • Removes interface libraries from IDE directory organization for compatibility with CMake<3.19

PR Checklist

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions bot added area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation labels Apr 6, 2022
@github-actions github-actions bot added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Apr 6, 2022
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Squa

Modules/Remote/VkFFTBackend.remote.cmake Show resolved Hide resolved
@tbirdso
Copy link
Contributor Author

tbirdso commented Apr 6, 2022

Seeing two different CMake errors on Windows vs Linux, not having much luck resolving either so far.

Windows (MSVC) generate error:

CMake Error at CMakeLists.txt:713 (get_target_property):
  INTERFACE_LIBRARY targets may only have whitelisted properties.  The
  property "SOURCE_DIR" is not allowed.
Call Stack (most recent call first):
  CMakeLists.txt:726 (itk_organize_targets)


CMake Error in C:/repos/ITK-build-remote-16/_deps/vulkan_lib-src/CMakeLists.txt:
  INTERFACE_LIBRARY targets may only have whitelisted properties.  The
  property "FOLDER" is not allowed.

Relevant CMakeLists.txt

Relevant CMake discussion shows the INTERFACE library property whitelist has been removed as of CMake 3.19, but ITK still supports CMake 3.16/17/18.

This code already runs on other ITK targets, unclear why the VkFFT dependency creates this issue? Perhaps no INTERFACE_LIBRARY targets?


Linux (Ninja) compile error:

FAILED: Modules/Remote/VkFFTBackend/test/CMakeFiles/VkFFTBackendTestDriver.dir/itkVkComplexToComplexFFTImageFilterTest.cxx.o
/usr/bin/c++  -I/home/tom/builds/ITK-remote/Modules/ThirdParty/Eigen3/src -I/home/tom/builds/ITK-remote/Modules/ThirdParty/KWSys/src -I/home/tom/repos/ITK/Modules/ThirdParty/VNL/src/vxl/v3p/netlib -I/home/tom/repos/ITK/Modules/ThirdParty/VNL/src/vxl/vcl -I/home/tom/repos/ITK/Modules/ThirdParty/VNL/src/vxl/core -I/home/tom/builds/ITK-remote/Modules/ThirdParty/VNL/src/vxl/v3p/netlib -I/home/tom/builds/ITK-remote/Modules/ThirdParty/VNL/src/vxl/vcl -I/home/tom/builds/ITK-remote/Modules/ThirdParty/VNL/src/vxl/core -I/home/tom/builds/ITK-remote/Modules/Core/Common -I/home/tom/repos/ITK/Modules/Core/Common/include -I/home/tom/repos/ITK/Modules/Filtering/ImageFilterBase/include -I/home/tom/repos/ITK/Modules/Filtering/ImageCompose/include -I/home/tom/repos/ITK/Modules/Core/ImageAdaptors/include -I/home/tom/builds/ITK-remote/Modules/ThirdParty/Netlib -I/home/tom/repos/ITK/Modules/Numerics/Statistics/include -I/home/tom/repos/ITK/Modules/Core/Transform/include -I/home/tom/repos/ITK/Modules/Core/ImageFunction/include -I/home/tom/repos/ITK/Modules/Filtering/ImageGrid/include -I/home/tom/builds/ITK-remote/Modules/ThirdParty/ZLIB/src -I/home/tom/builds/ITK-remote/Modules/ThirdParty/ZLIB/src/itkzlib-ng -I/home/tom/repos/ITK/Modules/ThirdParty/ZLIB/src -I/home/tom/builds/ITK-remote/Modules/ThirdParty/MetaIO/src/MetaIO/src -I/home/tom/repos/ITK/Modules/ThirdParty/MetaIO/src/MetaIO/src -I/home/tom/repos/ITK/Modules/Core/SpatialObjects/include -I/home/tom/repos/ITK/Modules/Filtering/ImageStatistics/include -I/home/tom/repos/ITK/Modules/Filtering/Path/include -I/home/tom/repos/ITK/Modules/Filtering/ImageIntensity/include -I/home/tom/repos/ITK/Modules/ThirdParty/DoubleConversion/src -I/home/tom/builds/ITK-remote/Modules/ThirdParty/DoubleConversion/src/double-conversion -I/home/tom/builds/ITK-remote/Modules/IO/ImageBase -I/home/tom/repos/ITK/Modules/IO/ImageBase/include -I/home/tom/repos/ITK/Modules/Core/TestKernel/include -I/home/tom/repos/ITK/Modules/Filtering/FFT/include -I/home/tom/repos/ITK/Modules/Filtering/ImageSources/include -I/home/tom/repos/ITK/Modules/Remote/VkFFTBackend/include -I/home/tom/repos/ITK/Modules/ThirdParty/VNL/src/vxl/core/vnl/algo -I/home/tom/repos/ITK/Modules/ThirdParty/VNL/src/vxl/core/vnl -I/home/tom/builds/ITK-remote/Modules/ThirdParty/VNL/src/vxl/core/vnl -I/home/tom/repos/ITK/Modules/ThirdParty/ZLIB/src/itkzlib-ng -isystem /home/tom/repos/ITK/Modules/ThirdParty/Eigen3/src/itkeigen/.. -mtune=generic -march=corei7 -Wall -Wcast-align -Wdisabled-optimization -Wextra -Wformat=2 -Winvalid-pch -Wno-format-nonliteral -Wpointer-arith -Wshadow -Wunused -Wwrite-strings -funit-at-a-time -Wno-strict-overflow -Wno-deprecated -Wno-invalid-offsetof -Woverloaded-virtual -Wstrict-null-sentinel  -msse2 -O3 -DNDEBUG -fPIE -std=c++14 -MD -MT Modules/Remote/VkFFTBackend/test/CMakeFiles/VkFFTBackendTestDriver.dir/itkVkComplexToComplexFFTImageFilterTest.cxx.o -MF Modules/Remote/VkFFTBackend/test/CMakeFiles/VkFFTBackendTestDriver.dir/itkVkComplexToComplexFFTImageFilterTest.cxx.o.d -o Modules/Remote/VkFFTBackend/test/CMakeFiles/VkFFTBackendTestDriver.dir/itkVkComplexToComplexFFTImageFilterTest.cxx.o -c /home/tom/repos/ITK/Modules/Remote/VkFFTBackend/test/itkVkComplexToComplexFFTImageFilterTest.cxx
In file included from /home/tom/repos/ITK/Modules/Remote/VkFFTBackend/include/itkVkComplexToComplexFFTImageFilter.h:24,
                 from /home/tom/repos/ITK/Modules/Remote/VkFFTBackend/test/itkVkComplexToComplexFFTImageFilterTest.cxx:19:
/home/tom/repos/ITK/Modules/Remote/VkFFTBackend/include/itkVkCommon.h:24:10: fatal error: vkFFT.h: No such file or directory
   24 | #include "vkFFT.h"
      |          ^~~~~~~~~
compilation terminated.

Headers should be included at ITKVkFFTBackend/CMakeLists.txt:61

Any thoughts?

EDIT: See InsightSoftwareConsortium/ITKVkFFTBackend#38 for Linux CMake fix in ITKVkFFTBackend.

EDIT: Resolved in recent updates

ITKVkFFTBackend and its VkFFT dependency rely on OpenCL definitions that are found in `FindOpenCL.cmake` as of CMake 3.10. Here we remove the outdated ITK version of `FindOpenCL.cmake` in favor of the newer version packaged with CMake.

See https://cmake.org/cmake/help/latest/module/FindOpenCL.html

VkFFT defines INTERFACE_LIBRARY targets which conflict with ITK's IDE
folder structuring helper for CMake<3.19. Amended to ignore
INTERFACE_LIBRARY targets in folder structuring.
@tbirdso
Copy link
Contributor Author

tbirdso commented Apr 7, 2022

@Leengit @thewtex @dzenanz Ping for re-review at your convenience

Also adding @jcfr for discussion of CMake changes

@tbirdso tbirdso requested a review from jcfr April 7, 2022 16:23
Copy link
Contributor

@Leengit Leengit left a comment

Choose a reason for hiding this comment

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

I know nothing about when ${type} would be "INTERFACE_LIBRARY" so I am no help with that code. Otherwise looks good.

@tbirdso
Copy link
Contributor Author

tbirdso commented Apr 7, 2022

For context, VkFFT generates two CMake "INTERFACE_LIBRARY" targets, VkFFT and half

CMake interface libraries are discussed here

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

🌮 🎉

@tbirdso tbirdso merged commit c49d637 into InsightSoftwareConsortium:master Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants