Skip to content

COMP: Fix FFTWF-only build on release-5.4#6326

Open
hjmjohnson wants to merge 5 commits into
ci-backport-rel54from
fix-fftwf-only-include-release-5.4
Open

COMP: Fix FFTWF-only build on release-5.4#6326
hjmjohnson wants to merge 5 commits into
ci-backport-rel54from
fix-fftwf-only-include-release-5.4

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson commented May 22, 2026

Fixes the FFTWF-only (single-precision FFTW) build on release-5.4. Stacked on release-5.4. Companion to #6330 (same fixes on main).

The two defects (masked by the default dual-precision build)
  1. CMake/itkExternal_FFTW.cmake set FFTW_INCLUDE from the double-precision FFTW3_INCLUDE_DIRS inside the ITK_USE_FFTWF block, leaving it empty in single-precision-only builds (fftw3.h not found).
  2. The FFTW factory instantiated both precisions. FFTImageFilterFactory<FFTWxxx> now consults per-precision predicates (FFTImageFilterEnableFloat/Double) guarded by if constexpr. Each FFTW filter header specializes the predicate to false_type for the absent precision, so the factory does not instantiate fftw::ComplexToComplexProxy<double> (undefined when ITK_USE_FFTWD=OFF).

The specializations live in each FFTW filter header (next to FFTImageFilterTraits<FFTWxxx>), not in a .cxx, so every translation unit that instantiates the factory — including itkTestDriverIncludeRequiredFactories.cxx — sees them. (This is the fix learned from #6330, where the .cxx-only specializations left other TUs broken.)

Local verification

Built ITKTestKernel on macOS arm64 with ITK_USE_FFTWF=ON / ITK_USE_FFTWD=OFF / ITK_USE_SYSTEM_FFTW=OFF against release-5.4: staged FFTW built its own fftw3.h, and itkTestDriverIncludeRequiredFactories.cxx compiles and links.

@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels May 22, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR fixes two bugs that caused build failures in FFTWF-only (single-precision) configurations on release-5.4, and temporarily adds a CI-validation commit to force the Pixi build matrix to exercise that path.

  • CMake fix: itkExternal_FFTW.cmake now correctly uses FFTW3f_INCLUDE_DIRS (not FFTW3_INCLUDE_DIRS) inside the ITK_USE_FFTWF block, so staged FFTW installs its fftw3.h on the include path in single-precision-only builds.
  • Factory fix: FFTImageFilterFactory gains FFTImageFilterEnableFloat/FFTImageFilterEnableDouble compile-time predicates guarded by if constexpr; each FFTW filter header specializes the absent precision to std::false_type, preventing instantiation of undefined fftw::ComplexToComplexProxy<double> when ITK_USE_FFTWD=OFF.
  • CurvatureRegistrationFilter: Guard narrowed from ITK_USE_FFTWF || ITK_USE_FFTWD to ITK_USE_FFTWD only, reflecting that its implementation uses fftw_plan (double-precision) exclusively; tests fall back from double to float pixel type under FFTWF-only builds.

Confidence Score: 4/5

The core C++ and CMake fixes are correct and safe to merge; the pyproject.toml change must be dropped first.

The CMake one-liner fix, the factory predicate mechanism, and the CurvatureRegistrationFilter guard narrowing are all sound. The only blocker is the pyproject.toml commit that hardcodes FFTWF-only flags into the default configure task — the author explicitly flags it as temporary and marks it for removal before the PR is marked ready. Merging without removing that commit would permanently alter the default Pixi developer build configuration for all contributors.

pyproject.toml — the WIP FFTW flags must be reverted before merging.

Important Files Changed

Filename Overview
CMake/itkExternal_FFTW.cmake Fixes the root CMake bug: FFTW3_INCLUDE_DIRS (double-precision) was incorrectly set for FFTW_INCLUDE inside the ITK_USE_FFTWF block; corrected to FFTW3f_INCLUDE_DIRS.
Modules/Filtering/FFT/include/itkFFTImageFilterFactory.h Adds FFTImageFilterEnableFloat/Double predicates (defaulting to true_type) and guards each registration block with if constexpr, preventing instantiation of undefined FFTW proxies when a precision is absent.
Modules/Filtering/FFT/include/itkFFTWComplexToComplexFFTImageFilter.h Adds FFTImageFilterEnableFloat/Double specializations to false_type guarded by preprocessor, and a direct fftw3.h include. Has one spurious extra blank line before namespace close.
Modules/Registration/PDEDeformable/include/itkCurvatureRegistrationFilter.h Correctly restricts the filter guard to ITK_USE_FFTWD only; removes the dead branch that declared RealTypeDFT=double while linking against the float library.
pyproject.toml Temporary WIP commit hardcoding FFTWF-only flags into the default configure task. PR author explicitly marks this as a drop-before-merge commit; merging as-is would permanently change the default Pixi build to single-precision-only.
Modules/Filtering/FFT/include/itkFFTWForwardFFTImageFilter.h Adds per-precision enable specializations and direct fftw3.h include, consistent with the pattern applied across all FFTW filter headers.
Modules/Filtering/FFT/test/itkFFT1DImageFilterTest.cxx Falls back from double to float pixel type when ITK_USE_FFTWD is not defined, allowing the test to run under FFTWF-only builds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[FFTImageFilterFactory constructor] --> B{FFTImageFilterEnableFloat::value?}
    B -->|true constexpr| C[Register float precision]
    B -->|false constexpr| D[Skip — no instantiation]
    A --> E{FFTImageFilterEnableDouble::value?}
    E -->|true constexpr| F[Register double precision]
    E -->|false constexpr| G[Skip — no instantiation]
    H[FFTW filter header] --> I{ITK_USE_FFTWF defined?}
    I -->|NO| J[EnableFloat specializes to false_type]
    I -->|YES| K[Inherits true_type default]
    H --> L{ITK_USE_FFTWD defined?}
    L -->|NO| M[EnableDouble specializes to false_type]
    L -->|YES| N[Inherits true_type default]
Loading

Reviews (2): Last reviewed commit: "COMP: Restrict CurvatureRegistrationFilt..." | Re-trigger Greptile

@github-actions github-actions Bot added the area:Filtering Issues affecting the Filtering module label May 22, 2026
@hjmjohnson hjmjohnson changed the base branch from main to release-5.4 May 22, 2026 17:57
@hjmjohnson
Copy link
Copy Markdown
Member Author

Companion CI-validation PR #6330 (WIP, targets main) exercises the FFTWF-only build path that this fix addresses — it forces ITK_USE_FFTWF=ON / ITK_USE_FFTWD=OFF on the Pixi-Cxx leg, which the default dual-precision matrix never covers.

@hjmjohnson hjmjohnson force-pushed the fix-fftwf-only-include-release-5.4 branch from 6620047 to 17509f0 Compare May 22, 2026 18:52
@hjmjohnson hjmjohnson changed the title BUG: Set FFTW_INCLUDE from FFTW3f_INCLUDE_DIRS in FFTWF-only builds WIP: Fix FFTWF-only build on release-5.4 (CI-validation commit temporary) May 22, 2026
@hjmjohnson hjmjohnson marked this pull request as draft May 22, 2026 18:52
@hjmjohnson
Copy link
Copy Markdown
Member Author

@dzenanz Build a 3rd-party app against this, identified an issue with placement of code in .cxx that needs to be in .h files.

Adding more CI testing (temporarily) to investigate the completeness of the solution.

@hjmjohnson hjmjohnson force-pushed the fix-fftwf-only-include-release-5.4 branch from 17509f0 to 231af23 Compare May 22, 2026 19:23
@github-actions github-actions Bot removed the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label May 22, 2026
@hjmjohnson hjmjohnson force-pushed the fix-fftwf-only-include-release-5.4 branch 2 times, most recently from faf8e60 to f070dd9 Compare May 23, 2026 19:23
@github-actions github-actions Bot added the type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct label May 23, 2026
@hjmjohnson hjmjohnson changed the title WIP: Fix FFTWF-only build on release-5.4 (CI-validation commit temporary) Fix FFTWF-only build on release-5.4 (CI-validation commit temporary) May 25, 2026
@github-actions github-actions Bot added the area:Registration Issues affecting the Registration module label May 25, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review May 25, 2026 15:46
Comment thread pyproject.toml Outdated
@hjmjohnson hjmjohnson changed the title Fix FFTWF-only build on release-5.4 (CI-validation commit temporary) COMP: Fix FFTWF-only build on release-5.4 May 26, 2026
@hjmjohnson hjmjohnson force-pushed the fix-fftwf-only-include-release-5.4 branch from a37dc47 to 5ee425b Compare May 26, 2026 17:02
@hjmjohnson
Copy link
Copy Markdown
Member Author

Dropped the WIP CI-forcing commit (f070dd9c77). The only resulting change vs. the approved revision is pyproject.toml reverting to the base configure task — the 5 fix commits are byte-identical. The FFTWF-only build is exercised by companion PR #6330.

@github-actions github-actions Bot added the type:Compiler Compiler support or related warnings label May 26, 2026
@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp run

In the ITK_USE_FFTWF block, FFTW_INCLUDE was assigned from
FFTW3_INCLUDE_DIRS (the double-precision variable), which is only set
inside the ITK_USE_FFTWD block. When ITK is configured with
ITK_USE_FFTWF=ON and ITK_USE_FFTWD=OFF, FFTW_INCLUDE is left empty, so
the staged fftw3.h include directory is never added and ITKFFT fails to
compile with "fatal error: 'fftw3.h' file not found".

The defect is masked whenever both precisions are enabled because the
FFTWD block reassigns FFTW_INCLUDE afterward. Use the single-precision
FFTW3f_INCLUDE_DIRS in the FFTWF block so single-precision-only builds
resolve the header.
FFTWFFTImageFilterInitFactory unconditionally registered every FFTW
filter for both float and double via FFTImageFilterFactory, whose
constructor instantiated both precisions. The single-precision-only
1-D FFTW filters (e.g. FFTWForward1DFFTImageFilter) form member type
aliases from fftw::ComplexToComplexProxy<PixelType>, which is defined
only for the configured precision. Building with ITK_USE_FFTWF=ON and
ITK_USE_FFTWD=OFF therefore failed to compile ITKFFT
("no type named 'PlanType' in 'itk::fftw::ComplexToComplexProxy<double>'").

Add FFTImageFilterEnableFloat / FFTImageFilterEnableDouble predicates
(both default true) and guard the factory constructor with if constexpr.
The FFTW InitFactory specializes the predicate to false_type for the
precision whose backend is absent, so only the configured precision is
registered. Vnl and other factories are unaffected because the
predicates default to enabling both precisions.
The FFTW filter headers reference FFTW_FORWARD/FFTW_BACKWARD/FFTW_ESTIMATE
and other fftw3.h symbols but obtained them only transitively through
itkFFTWCommon.h / itkFFTWCommonExtended.h. Those proxy headers guard the
fftw3.h include with `#if defined(ITK_USE_FFTWF) || defined(ITK_USE_FFTWD)`
without first including itkConfigure.h. When a proxy header is first
included before ITK_USE_FFTW* is defined, the guard is false, fftw3.h is
skipped, and the proxy's include guard is set so later inclusions are
no-ops, leaving the FFTW symbols undeclared at parse time (observed in
ITKFFTHeaderTest1 with GCC for single-precision-only builds).

Include fftw3.h directly, guarded, in each consuming header so the
symbols are visible regardless of header include order.
The 1D FFT tests hardcoded double, which fails to compile in an
ITK_USE_FFTWF=ON, ITK_USE_FFTWD=OFF build because only
ComplexToComplexProxy<float> defines PlanType/ComplexType. Select the
pixel type from the configured precision so the FFTW backend
instantiates a proxy that exists.

Backport of the single-precision FFTW 1D-test pixel-type fix from main
(PR #6330). The default-backend (Vnl) check is unchanged on release-5.4.
itkCurvatureRegistrationFilter uses FFTW real-to-real (DCT) transforms
implemented only for double precision, so it requires ITK_USE_FFTWD.
The prior guard also admitted ITK_USE_FFTWF-only builds, whose
single-precision branch emitted a #warning that MSVC rejects as fatal
error C1021, breaking the single-precision-only Windows build.
@hjmjohnson hjmjohnson force-pushed the fix-fftwf-only-include-release-5.4 branch from 5ee425b to 2477dcc Compare May 26, 2026 20:35
@hjmjohnson hjmjohnson changed the base branch from release-5.4 to ci-backport-rel54 May 26, 2026 20:35
@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp run ITK.macOS.Python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants