ENH: Refactor FFT classes for object factory registration

[tbirdso]

Takes the object factory structure introduced to itk::Forward1DFFTImageFilter in #2836 and extends it to all FFT classes.

Relying on object factories for generating FFT classes will allow users to flexibly override FFT implementations so that their default backend is the one best for their own system. Developers can develop against base classes such as itk::ComplexToComplexFFTImageFilter, itk::HalfHermitianToRealInverseFFTImageFilter, etc, without needing to know whether the FFT backend is Vnl, FFTW, or something else.

Prior to these changes backend selection was performed with dispatch methods overriding base constructors, which locked in available backends to those included with ITK proper. Moving to the object factory framework will allow remote modules to inject accelerated backend defaults, such as in the case of https://github.com/InsightSoftwareConsortium/ITKVkFFTBackend.

Consuming projects must use include(UseITK.cmake) to properly initialize default object factory registrations. Discussion in #2836 concluded that ITK tests do not include this line and therefore do not register factories by default, hence filtering cxx tests relying on arbitrary FFT classes have been refactored to include manual object factory registrations.

Python wrapping does include UseITK.cmake and default registers factories, therefore manual registrations are not required in ITK Python testing but may be required for remote module consumption. This will be investigated with changes to igenerator.py in a subsequent PR. Further discussion on object factory architecture and behavior can be found at https://discourse.itk.org/t/proposal-to-improve-itk-factory-registration-cmake-infrastructure/4546/4.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5

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

[dzenanz]

Thanks for this contribution. My main concern is duplication of repetitive code. That can probably be solved via a macro, unless there is a more elegant solution.

[dzenanz]

When this compiles, convert it back into a ready PR. If you want me to review before then, ping me.

[Leengit]

Looking good. A few comments about the FFTImageFilterTraits.

[tbirdso]

After resolving several compile/test issues I'm still seeing a strange failure in Python tests.

CDash shows that PythonGetNameOfClass and three other Python tests are failing with a segfault:

itkTestDriver: Process exception: Segmentation fault

I've traced this back to itk.FFTConvolutionImageFilter.New(), which attempts to instantiate an itk.RealToHalfHermitianFFTImageFilter object in its constructor in order to set a member:

template <typename TInputImage, typename TKernelImage, typename TOutputImage, typename TInternalPrecision>
FFTConvolutionImageFilter<TInputImage, TKernelImage, TOutputImage, TInternalPrecision>::FFTConvolutionImageFilter()
{
  m_SizeGreatestPrimeFactor = FFTFilterType::New()->GetSizeGreatestPrimeFactor();
}

However, FFTFilterType::New() returns a nullptr and the subsequent function call on the nullptr causes the segfault. This implies that the FFT factory is not properly registered before the call executes.

Factories should be registered in Python by default, especially given that force_load() is called at test initialization. On the command line it seems that factories are indeed registered, as instantiating with itk.<FFT class>.New() and then casing to itk.<Vnl FFT class> succeeds. However, calling itk.FFTConvolutionImageFilter[<types>].New() always fails.

Investigation notes:

• Adding printouts to itkVnlFFTImageFilterFactories.cxx shows that FFT factories are indeed registered at load time as expected
• Trying to replace itk::RealToHalfHermitianForwardFFTImageFilter with any other FFT type in the FFTConvolutionImageFilter constructor fails and returns nullptr. No FFT factories seem to be present (or at least accessible) from the constructor.
• FFTConvolutionImageFilter is wrapped with TInternalPrecision = double, which propagates to an attempt to instantiate the forward FFT filter with double type, which is not explicitly wrapped. However, 1) it's not required for classes used internally to be wrapped for the external filter to be used in Python, 2) FFT image filters of double pixel type should be registered alongside float types by default, and 3) changing the default to TInternalPrecision = float does not resolve the issue.
• Trying to check factory registrations with itk.ObjectFactoryBase.GetRegisteredFactories() returns an empty tuple, which is unexpected as this should be a singleton against which all factories are registered. After calling itk.imread to register an IO factory itk.ObjectFactoryBase.GetRegisteredFactories() still returns the empty tuple. Not sure exactly what's happening here.
• Subclasses of FFTConvolutionImageFilter also segfault during construction (which is not unexpected)
• Registering the required FFT factory at the start of the constructor results in an FFT filter being appropriately constructed. The convolution filter is constructed with no segfault.

Open to suggestions on how to proceed here. Would like to avoid adding dynamic Python factory registration expansions to this PR as it's already very large, but will reluctantly head that way if that is the required resolution.

[dzenanz]

Maybe you can squash the changes so far?

[Leengit]

In case it is not a red herring, ... in #2890 there is a short discussion that might explain why factories that are registered don't appear that way to other parts of the code. It includes

It has to do with ITK python building the ITK libraries static, but having each ITK module as a separate loadable (Shared library) python module. The there are multiple instances of these "global" variables on some system.

[tbirdso]

Thanks @Leengit , this sounds like the most likely cause right now. I'll have to look more into how the GlobalTimestamp singleton is unified across Python modules and see if that can be applied here.

[thewtex]

Some series templating here! 🕺

@tbirdso do these examples still work when built against this branch?

https://itk.org/ITKExamples/src/Filtering/FFT/index.html

[tbirdso]

Hi @thewtex , yes, the FFT examples still work when built against this branch on Windows. I'll also note that instantiating FFTConvolutionImageFilter in Python works on Windows as well, but not on Linux.

I'm working on building the examples in Linux, in the meantime I've submitted a small path for the inverse FFT example here: InsightSoftwareConsortium/ITKSphinxExamples#312

[tbirdso]

Verified that Linux FFT examples build. So far the problem seems limited to using object factories in consuming modules in Python on Linux, i.e. using FFT object factories in Convolution filters. The discussion in #2890 in regards to magic statics may be the root of the issue, though I'm not clear at the moment why this would be platform-dependent behavior.

[tbirdso]

Current hypothesis: registering FFT factories with "magic statics" in itk::ObjectFactoryBase::RegisterInternalFactoryOnce confines registration to the object factory "singleton" in the ITKFFT Python module which is not visible from the ITKConvolution module.

#2890 dealing with magic statics was abandoned (at least for now) due to itkSingletonMacro being required for synchronizing singletons across ITK Python modules. Meanwhile, #2819 recently introduced magic statics with RegisterInternalFactoryOnce for IO object factory thread safety. This is reasonable for IO factories because they are only called within the ITKIO module. However, we can't extend that design pattern here because we expect filters outside of ITKFFT will make use of FFT filters internally.

In an initial test registering itk::VnlRealToHalfHermitianForwardFFTImageFilter with a static HasBeenRegistered variable (per the design pattern prior to #2819) I am able to successfully instantiate an itk::FFTConvolutionImageFilter which instantiates an itk::RealTohalfHermitianForwardFFTImageFilter via the object factory in its constructor in Python on Linux. This addresses the remaining Python issues discussed above; I will add a draft commit along these lines.

As I see it there are two ways to proceed here:

• Further investigate global singleton synchronization across ITK modules as is done with TimeStamp::Modified as discussed in PERF: Use "magic statics" instead of Singleton in TimeStamp::Modified() #2890. It's not clear to me how much additional effort this will add.
• Remove magic statics from FFT factory registration and rely on static booleans to ensure that only one factory is registered. This exposes potential issues with thread safety and race conditions where a factory could be registered multiple times at initialization; however, it's not clear that this will occur in practice. I would favor moving forward with this approach and reconsidering if we see issues during testing.

I'm certainly open to other ideas here as well. Pinging @N-Dekker and @thewtex for discussion of object factories + magic statics.

[tbirdso]

Marking as ready for review but intentionally leaving the last commit unsquashed for design discussion.

[tbirdso]

Per discussion with @thewtex we will leave FFT registration as-is for now, but follow up with changes to synchronize itk::ObjectFactoryBase across Python modules as we do with the timestamp singleton. Those changes will open up object factory registration for custom backends across ITK classes going forward.

[tbirdso]

For future reference, reading a few lines into documentation on C APIs for Python extension modules reveals a likely answer for why FFT factories registered with magic statics were visible across modules in Windows but not in Linux:

When modules are used as shared libraries, however, the symbols defined in one module may not be visible to another module. The details of visibility depend on the operating system; some systems use one global namespace for the Python interpreter and all extension modules (Windows, for example), whereas others require an explicit list of imported symbols at module link time (AIX is one example), or offer a choice of different strategies (most Unices).

[thewtex]

⚒️ great progress!!

I created an issue to track the next step of object factory resolution in #2909