FFT Python wrappings don't compile

dzenanz commented 2 years ago

Description

I get errors when building Python wrapping:

74>------ Build started: Project: ITKFFTCastXML, Configuration: RelWithDebInfo x64 ------
74>Generating ../../itkVnlComplexToComplex1DFFTImageFilter.xml
74>C:/Dev/ITK-py/Wrapping/itkVnlComplexToComplex1DFFTImageFilter.cxx(26,18): error G026DE12C: too few template arguments for class template 'VnlComplexToComplex1DFFTImageFilter'
74>    typedef itk::VnlComplexToComplex1DFFTImageFilter< itk::Image< std::complex< float >,2 > > itkVnlComplexToComplex1DFFTImageFilterICF2;
74>                 ^
74>C:/Dev/ITK-git/Modules/Remote/Ultrasound/include\itkVnlComplexToComplex1DFFTImageFilter.h:35:27: note: template is declared here
74>class ITK_TEMPLATE_EXPORT VnlComplexToComplex1DFFTImageFilter
74>                          ^
74>C:/Dev/ITK-py/Wrapping/itkVnlComplexToComplex1DFFTImageFilter.cxx(27,18): error G026DE12C: too few template arguments for class template 'VnlComplexToComplex1DFFTImageFilter'
74>    typedef itk::VnlComplexToComplex1DFFTImageFilter< itk::Image< std::complex< float >,2 > >::Pointer itkVnlComplexToComplex1DFFTImageFilterICF2_Pointer;
74>                 ^
74>C:/Dev/ITK-git/Modules/Remote/Ultrasound/include\itkVnlComplexToComplex1DFFTImageFilter.h:35:27: note: template is declared here
74>class ITK_TEMPLATE_EXPORT VnlComplexToComplex1DFFTImageFilter
74>                          ^
74>C:/Dev/ITK-py/Wrapping/itkVnlComplexToComplex1DFFTImageFilter.cxx(29,18): error G026DE12C: too few template arguments for class template 'VnlComplexToComplex1DFFTImageFilter'
74>    typedef itk::VnlComplexToComplex1DFFTImageFilter< itk::Image< std::complex< float >,3 > > itkVnlComplexToComplex1DFFTImageFilterICF3;
74>                 ^
74>C:/Dev/ITK-git/Modules/Remote/Ultrasound/include\itkVnlComplexToComplex1DFFTImageFilter.h:35:27: note: template is declared here
74>class ITK_TEMPLATE_EXPORT VnlComplexToComplex1DFFTImageFilter
74>                          ^
74>C:/Dev/ITK-py/Wrapping/itkVnlComplexToComplex1DFFTImageFilter.cxx(30,18): error G026DE12C: too few template arguments for class template 'VnlComplexToComplex1DFFTImageFilter'
74>    typedef itk::VnlComplexToComplex1DFFTImageFilter< itk::Image< std::complex< float >,3 > >::Pointer itkVnlComplexToComplex1DFFTImageFilterICF3_Pointer;
74>                 ^
74>C:/Dev/ITK-git/Modules/Remote/Ultrasound/include\itkVnlComplexToComplex1DFFTImageFilter.h:35:27: note: template is declared here
74>class ITK_TEMPLATE_EXPORT VnlComplexToComplex1DFFTImageFilter
74>                          ^
74>4 errors generated.
74>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for 'C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlComplexToComplex1DFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlComplexToComplexFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlForward1DFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlForwardFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlHalfHermitianToRealInverseFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlInverse1DFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlInverseFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\e7eae92ef42e7e26c5c7fa0a76b60a70\itkVnlRealToHalfHermitianForwardFFTImageFilter.xml.rule;C:\Dev\ITK-py\CMakeFiles\32c6fec31d408514add2e7156be2d650\ITKFFTCastXML.rule' exited with code 1.
74>Done building project "ITKFFTCastXML.vcxproj" -- FAILED.

Steps to Reproduce

Incremental compile after updating local ITK repository.

Expected behavior

Build completes with no errors.

Actual behavior

Builds error above.

Reproducibility

100%

Versions

Recent master, e.g. c46354736b0723ae4a7c66b03b6203c643754659.

Environment

Windows 10, VS2019.

Additional Information

@nicktasios also encountered as similar problem:

[ 84%] Generating itkForward1DFFTImageFilterPython.cpp, ../../Generators/Python/itk/itkForward1DFFTImageFilterPython.py                                                               
/home/username/path/ITKPythonPackage/_skbuild/linux-x86_64-3.7/cmakebuild/ITKb/Wrapping/Typedefs/itkForward1DFFTImageFilter.i:82: Warning 401: Nothing known about base class 'itk::ImageToImageFilter< itk::Image< double,2 >,itk::Image< std::complex< double >,2 > >'. Ignored.                                                                           
/home/username/path/ITKPythonPackage/_skbuild/linux-x86_64-3.7/cmake-build/ITKb/Wrapping/Typedefs/itkForward1DFFTImageFilter.i:82: Warning 401: Maybe you forgot to instantiate 'itk::ImageToImageFilter< itk::Image< double,2 >,itk::Image< std::complex< double >,2 > >' using %template.                                                                   
gmake[5]: *** [Wrapping/Modules/ITKFFT/CMakeFiles/ITKFFTPython.dir/build.make:1738: Wrapping/Modules/ITKFFT/itkForward1DFFTImageFilterPython.cpp] Error 2                             
gmake[4]: *** [CMakeFiles/Makefile2:25792: Wrapping/Modules/ITKFFT/CMakeFiles/ITKFFTPython.dir/all] Error 2                                                                           
gmake[3]: *** [Makefile:172: all] Error 2                                                                                                                                             
gmake[2]: *** [CMakeFiles/ITK.dir/build.make:132: ITKp/src/ITK-stamp/ITK-build] Error 2                                                                                               
gmake[1]: *** [CMakeFiles/Makefile2:129: CMakeFiles/ITK.dir/all] Error 2                                                                                                              
gmake: *** [Makefile:150: all] Error 2

thewtex commented 2 years ago

ITK-git/Modules/Remote/Ultrasound

We need to update the ITKUltrasound remote to 5.3rc3 once released.

tbirdso commented 2 years ago

A few initial thoughts here:

As discussed in https://github.com/KitwareMedical/ITKUltrasound/pull/161, echoing what @thewtex said, 1DFFT classes are still present in ITKUltrasound and will not be removed until ITK v5.3rc3 is released, because they were moved to ITK proper after the v5.3rc2 release and are not yet available in the ITK Python distribution on PyPi.
CDash and recent PRs have shown ITK Python builds passing. @dzenanz could you provide more context surrounding your local development environment? Most likely an issue with remote module compatibility?
In bb3c27e the second template parameter in itkVnlComplexToComplex1DFFTImageFilter was made optional as part of standardizing FFT class signatures. It looks like that change may not be present in your source tree?

74>Generating ../../itkVnlComplexToComplex1DFFTImageFilter.xml 74>C:/Dev/ITK-py/Wrapping/itkVnlComplexToComplex1DFFTImageFilter.cxx(26,18): error G026DE12C: too few template arguments for class template 'VnlComplexToComplex1DFFTImageFilter' 74> typedef itk::VnlComplexToComplex1DFFTImageFilter< itk::Image< std::complex< float >,2 > > itkVnlComplexToComplex1DFFTImageFilterICF2;

I'm less confident that the issue in #2884 is related as ImageToImage real<->complex wrapping signatures have been included in itkImageToImageFilterB.wrap long prior to FFT changes. If we're not able to resolve the issue in that thread I would recommend tracking in a separate issue. EDIT: On second thought, this might indicate that the filter is trying to wrap for double and complex<double> types even though ITK_WRAP_DOUBLE and ITK_WRAP_COMPLEX_DOUBLE should not be on by default. EDIT2: itkForward1DFFTImageFilter.wrap shows that the FFT filter is wrapped for input images of pixel type T = WRAP_ITK_REAL with a default output image type std::complex<T>. Is it possible that ITK_WRAP_DOUBLE is enabled but ITK_WRAP_COMPLEX_DOUBLE is disabled by default when trying to build with setup.py?

tbirdso commented 2 years ago

Regardless of whether my suggestion in #2884 is the correct fix, we should certainly be checking whether complex types are available in wrapping before trying to wrap relevant FFT classes. Will put in a fix shortly.

dzenanz commented 2 years ago

In bb3c27e the second template parameter in itkVnlComplexToComplex1DFFTImageFilter was made optional as part of standardizing FFT class signatures. It looks like that change may not be present in your source tree?

This is present in my source tree. I was not sure whether this template default would propagate to derived classes.

No variant of double is enabled for wrapping in the build in question. I see 4 options total: plain, complex, vector and covariant vector.

tbirdso commented 2 years ago

This is present in my source tree. I was not sure whether this template default would propagate to derived classes.

Okay, thanks for checking. Is the Ultrasound remote module enabled in your build? Given that the second template argument was made optional only after moving 1D FFT classes into ITK proper, perhaps the lingering, original structure from ITKUltrasound is being picked up?

No variant of double is enabled for wrapping in the build in question. I see 4 options total: plain, complex, vector and covariant vector.

🤔 I'm not sure how to trace this issue, as itkForward1DFFTImageFilter.wrap seems to only wrap for the pixel types present in WRAP_ITK_REAL and I haven't been able to recreate locally or in CI. Are you building on Windows @dzenanz ? Perhaps it's worth putting in the check for complex types being enabled in wrapping and then revisiting if the issue still appears? Or as I mentioned in #2884, trying to enable double and complex<double> types and seeing what changes?

dzenanz commented 2 years ago

trying to enable double and complex types and seeing what changes

double is wrapped by default in PythonPackage repository, but not in vanilla ITK. Whoever tries to build Python wrappings from source just by enabling ITK_WRAP_PYTHON might run into the issue.

perhaps the lingering, original structure from ITKUltrasound is being picked up

Oops, I had Module_Ultrasound enabled. I will try building incrementally without it.

tbirdso commented 2 years ago

Oops, I had Module_Ultrasound enabled. I will try building incrementally without it.

No problem, and sounds good. My hope is that this will resolve your issue, let me know if you are still seeing the error once it builds.

double is wrapped by default in PythonPackage repository, but not in vanilla ITK. Whoever tries to build Python wrappings from source just by enabling ITK_WRAP_PYTHON might run into the issue.

💯 Thanks for the find @dzenanz , ITKPythonPackage/CMakeLists.txt turns on several default values including ITK_WRAP_DOUBLE but not ITK_WRAP_COMPLEX_DOUBLE, which explains the issue in #2884. I propose two simultaneous fixes:

Check for the existence of complex types before wrapping FFT classes such as in itkForward1DFFTImageFilter.wrap, and
Turn on ITK_WRAP_COMPLEX_DOUBLE by default in ITKPythonPackage

@dzenanz @thewtex do these fixes sound good to you?

dzenanz commented 2 years ago

An incremental build succeeded with Module_Ultrasound disabled.

The changes you propose sound good.

InsightSoftwareConsortium / ITK