ENH: Ingest ITKSmoothingRecursiveYvvGaussianFilter into Modules/Filtering#6243
Conversation
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Error lim for floats. Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
…ursiveYvv GaussianFilter
…ursiveYvv GaussianFilter
…ursiveYvv GaussianFilter
Smaller test images. Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Following the new naming rules for ITK remote modules, the remote module name should not contain the "ITK" prefix.
STYLE: Remove "ITK" from the module name.
EXCLUDE_FROM_ALL is replaced by EXCLUDE_FROM_DEFAULT in ITK commit:0a98a978509a9536e07c22cde254559cc3c7d417
FIX: ITK warnings of deprecated EXCLUDE_FROM_ALL.
Signed-off-by: Irina Vidal <irina.vidal-migallon@inria.fr>
Also clean up itkYvvWhiteImageTest.cxx to have proper whitespace.
COMP: Fix compliler warnings
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
Signed-off-by: Irina Vidal Migallon <irina_ext@maunakeatech.com>
This is required when building against an install tree with GPU support to avoid linking errors.
BUG: Add itk_module_target call for library.
Additional Files for Python Wrapping
expression This check is responsible for using the auto type specifier for variable declarations to improve code readability and maintainability. The auto type specifier will only be introduced in situations where the variable type matches the type of the initializer expression. In other words auto should deduce the same type that was originally spelled in the source
The mission of NumFOCUS is to promote open practices in research, data, and scientific computing. https://numfocus.org
warning: dynamic exception specifications are deprecated in C++11 [-Wdeprecated]
…wareConsortium/github-actions
Fixes changes made in InsightSoftwareConsortium#2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
A ';' is required at the end of macros in a future version of ITK.
The ability to include either .h or .hxx files as header files required recursively reading the .h files twice. The added complexity is unnecessary, costly, and can confuse static analysis tools that monitor header guardes (due to reaching the maximum depth of recursion limits for nested #ifdefs in checking).
Replace add_library with itk_module_add_library macro for better integration with ITK module system. This provides automatic dependency linking, include directory setup, and consistent target naming.
…twareConsortium/modernize-cmake-it k-module-add-library ENH: Modernize CMake to use itk_module_add_library
…ring Mode-A merge of upstream ITKSmoothingRecursiveYvvGaussianFilter into ITK proper at Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/, ingested via the v4 pipeline (whitelist filter-repo + per-blob sanitize + no-ff merge). The filter implements Vliet/Young/Verbeek's recursive Gaussian smoothing approximation as a drop-in alternative to the canonical itk::SmoothingRecursiveGaussianImageFilter. GPU classes are gated on ITK_USE_GPU. Upstream: https://github.com/InsightSoftwareConsortium/ITKSmoothingRecursiveYvvGaussianFilter The upstream repository will be archived read-only after this PR merges; it remains reachable at the URL above for historical reference. The upstream top-level CMakeLists.txt's bespoke OpenCL detection is intentionally not migrated; a clean three-line in-tree CMakeLists.txt is added in a follow-up commit.
…Lists.txt The upstream top-level CMakeLists.txt carried bespoke find_package(OpenCL) + try_run detection that is unsuitable for an in-tree ITK module; replace with the canonical three-line project() + itk_module_impl() form. GPU sources continue to build under ITK_USE_GPU via src/CMakeLists.txt.
|
@greptile review |
|
| Filename | Overview |
|---|---|
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/include/itkGPUSmoothingRecursiveYvvGaussianImageFilter.hxx | GPU filter computes Gaussian b-coefficients from output spacing at SetSigma time (typically default 1.0), not actual input spacing; produces incorrect kernel for non-unit-spacing images |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/include/itkGPUSmoothingRecursiveYvvGaussianImageFilter.h | Class declaration with std::vector members replacing raw new[] (fixed in 53f020e); gated behind #ifdef GPU and ITK_USE_GPU correctly |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/include/itkSmoothingRecursiveYvvGaussianImageFilter.h | CPU outer filter declaration; three typedef declarations should be using aliases to match the rest of the file; otherwise clean |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/include/itkRecursiveLineYvvGaussianImageFilter.hxx | CPU line filter correctly calls SetUp() with actual input spacing in BeforeThreadedGenerateData; raw new[]/delete[] in ThreadedGenerateData are correctly paired for ProcessAborted but would leak on unexpected exceptions |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/itk-module.cmake | Module dependencies correctly declared with ITKGPUSmoothing and ITKGPUCommon gated on ITK_USE_GPU; typo GPU_DEPENDANCIES fixed in 53f020e |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/src/CMakeLists.txt | Correctly compiles only GPU kernel sources under ITK_USE_GPU; undefined-variable target_link_libraries block and unreachable guards dropped in 53f020e |
| Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/test/CMakeLists.txt | add_definitions(-DWITH_DOUBLE) in non-GPU path means tests compile with double precision while the production code uses float, creating a precision mismatch between tests and real usage |
| Modules/Remote/SmoothingRecursiveYvvGaussianFilter.remote.cmake | Remote module descriptor correctly removed now that the module is in-tree under Modules/Filtering/ |
| pyproject.toml | Module_SmoothingRecursiveYvvGaussianFilter:BOOL=ON correctly added to configure-ci cmake flags |
Sequence Diagram
sequenceDiagram
participant User
participant GPUFilter as GPUSmoothingRecursiveYvvGaussianImageFilter
participant Output as OutputDataObject
participant GPUKernel as OpenCL Kernel
User->>GPUFilter: SetSigma(sigma)
GPUFilter->>Output: GetSpacing() returns [1.0, 1.0] default pre-pipeline
GPUFilter->>GPUFilter: "SetUp(spacing=1.0) coefficients computed HERE"
Note over GPUFilter: b0,b1,b2,b3,M-matrix frozen with spacing=1.0
User->>GPUFilter: "SetInput(imageWithSpacing=0.5mm)"
User->>GPUFilter: Update()
GPUFilter->>GPUFilter: GPUGenerateData()
Note over GPUFilter: BuildKernel() called once no SetUp() call
GPUFilter->>GPUKernel: "Launch with stale coefficients spacing=1.0"
Note over GPUKernel: Wrong Gaussian kernel for 0.5mm spacing
Note right of GPUFilter: CPU path BeforeThreadedGenerateData calls SetUp(actualSpacing)
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile
Greptile P1/P2 findings:
- itk-module.cmake: rename `GPU_DEPENDANCIES` -> `GPU_DEPENDENCIES`
(typo).
- src/CMakeLists.txt: drop `${INRIA_ITK_LIBRARIES}` /
`${OPENCL_LIBRARIES}` (undefined in ITK; OpenCL linkage is provided
by the `ITKGPUCommon` module dep). Drop the `GPU_HANDLES_DOUBLE` /
`NVIDIA_GPU` guards (variables are never set in ITK's build, so the
branches were unreachable).
- test/CMakeLists.txt: same `GPU_HANDLES_DOUBLE` / `NVIDIA_GPU` guard
removal.
- src/OpenCLInfo.cpp: remove the standalone-only diagnostic utility
(it has its own `main()` and is not listed in src/CMakeLists.txt).
- include/itkGPUSmoothingRecursiveYvvGaussianImageFilter.h{,xx}:
replace raw `new ScalarRealType[N]` for `m_Bvalues` and
`m_CPUMatrix` with `std::vector<ScalarRealType>`. The previous code
leaked on every `SetUp()` re-entry (the destructor is `= default`
and there is no matching `delete[]`). `SetCPUBufferPointer` now
uses `.data()`.
# Conflicts: # pyproject.toml
| GPUSmoothingRecursiveYvvGaussianImageFilter<TInputImage, TOutputImage>::SetSigmaArray(const SigmaArrayType & sigma) | ||
| { | ||
| if (this->m_Sigma != sigma) | ||
| { | ||
| this->m_Sigma = sigma; | ||
|
|
||
| const typename InputImageType::SpacingType & pixelSize = this->GetOutput()->GetSpacing(); | ||
| if (pixelSize[0] != 0) | ||
| { | ||
| this->SetUp(pixelSize[0]); | ||
| } | ||
| this->Modified(); | ||
| } | ||
| } |
There was a problem hiding this comment.
GPU Gaussian coefficients computed with wrong spacing for non-unit-spacing images
SetSigmaArray reads spacing from this->GetOutput()->GetSpacing() (line 195). Before Update() is called, the output DataObject carries default ITK spacing (1.0 in every dimension), not the actual input image spacing. Since GPUGenerateData() never calls SetUp() again, the b-coefficients and M-matrix are frozen at the values computed with spacing=1.0. For any real-world image where pixel spacing != 1.0 mm (e.g., CT with 0.5 mm voxels), the GPU path silently produces an incorrect Gaussian kernel while the CPU sibling (RecursiveLineYvvGaussianImageFilter::BeforeThreadedGenerateData, line 288) correctly calls this->SetUp(pixelSize[m_Direction]) with the actual input spacing every time the filter runs.
Ingest the standalone remote module
InsightSoftwareConsortium/ITKSmoothingRecursiveYvvGaussianFilterinto
Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/. Vliet/Young/Verbeekrecursive Gaussian smoothing — drop-in alternative to the canonical
itk::SmoothingRecursiveGaussianImageFilter. In-tree underEXCLUDE_FROM_DEFAULT. GPU classes are gated onITK_USE_GPU.Built directly on
upstream/mainvia the v4 ingestion pipeline (#6204).Commits
ENH: Ingest ITKSmoothingRecursiveYvvGaussianFilter into Modules/Filtering(Mode-A merge)DOC: Add SmoothingRecursiveYvvGaussianFilter README and in-tree CMakeLists.txtCOMP: Remove SmoothingRecursiveYvvGaussianFilter .remote.cmake (in-tree)ENH: Enable Module_SmoothingRecursiveYvvGaussianFilter in configure-ciSTYLE: gersemi-format SmoothingRecursiveYvvGaussianFilter src/CMakeLists.txtMerge topology preserved: 24 merge commits, 87 commits unique to this PR.
OpenCL handling
The upstream top-level
CMakeLists.txtcarried bespokefind_package(OpenCL)+try_rundetection that conflicts with ITK'sin-tree GPU support (
ITKGPUCommon). It is intentionally not migrated;a clean three-line
project()+itk_module_impl()form takes itsplace. GPU sources continue to build under
ITK_USE_GPUviasrc/CMakeLists.txt.Phase B follow-up
After merge, archive the upstream repo via the v4 archive script: