Skip to content

DOC: Update FFTPadImageFilter prime factor doc for PocketFFT#6574

Draft
blowekamp wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
blowekamp:docFFTPadPocketFFT
Draft

DOC: Update FFTPadImageFilter prime factor doc for PocketFFT#6574
blowekamp wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
blowekamp:docFFTPadPocketFFT

Conversation

@blowekamp

Copy link
Copy Markdown
Member

Add PocketFFT (11) to the SizeGreatestPrimeFactor default-value list in FFTPadImageFilter; fix minor typo ("values is" → "value is").

AI assistance

GitHub Copilot located the PocketFFT GetSizeGreatestPrimeFactor return value (11) and drafted the comment update.

@github-actions github-actions Bot added type:Documentation Documentation improvement or change area:Filtering Issues affecting the Filtering module labels Jul 9, 2026
@blowekamp

Copy link
Copy Markdown
Member Author

The read me for pocket fft has the following:

Efficient codelets are available for the factors:

2, 3, 4, 5, 7, 8, 11 for complex-valued FFTs
2, 3, 4, 5 for real-valued FFTs

There was also some uncertainty if the FFTW value was correct too. This doc says it is only optimized up to 7: https://www.fftw.org/fftw2_doc/fftw_1.html

@hjmjohnson hjmjohnson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One correction and one adjacent cleanup, both as one-click suggestions. (Supersedes my earlier approval.)

Verified values from the code on main
  • FFTWProxyType::GREATEST_PRIME_FACTOR is 13 (7 when built with ITK_USE_CUFFTW) — itkFFTWCommon.h. The fftw2 doc reference predates FFTW3's codelet set; ITK's constant is what this doc should describe.
  • All PocketFFT filters (including the real-valued RealToHalfHermitian/HalfHermitianToReal variants) return 11 from GetSizeGreatestPrimeFactor(). If the real-valued paths should return 5 per the pocketfft README, that is a code change to make first — this doc should match what the code returns today.
  • The VNL/Temperton FFT backend was removed; the Vnl*FFTImageFilter names are now deprecated aliases deriving from the PocketFFT filters. They do not override GetSizeGreatestPrimeFactor(), so they inherit and report 11, never 5.

Comment on lines +85 to 86
* obtained from the ForwardFFTImageFilter, which is 13 for FFTW, 11 for PocketFFT,
* and 5 for VNL.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The VNL/Temperton backend was removed; the deprecated Vnl* aliases derive from the PocketFFT filters and inherit 11, so "5 for VNL" is no longer reachable.

Suggested change
* obtained from the ForwardFFTImageFilter, which is 13 for FFTW, 11 for PocketFFT,
* and 5 for VNL.
* obtained from the ForwardFFTImageFilter, which is 13 for FFTW (7 with cuFFTW)
* and 11 for PocketFFT.

* obtained from the ForwardFFTImageFilter, which is 13 for FFTW, 11 for PocketFFT,
* and 5 for VNL.
*
* A greatest prime factor of 2 produce a size which is a power of 2, and thus

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Grammar, and the next line's "is suitable for vnl base fft filters." is also stale (VNL backend removed) — it is just outside this hunk, so please fold it in manually, e.g. end the sentence at "power of 2." and drop the VNL clause.

Suggested change
* A greatest prime factor of 2 produce a size which is a power of 2, and thus
* A greatest prime factor of 2 produces a size which is a power of 2, and thus

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 type:Documentation Documentation improvement or change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants