Skip to content

ENH: Ingest ITKMorphologicalContourInterpolation (stacked on #6208) - #6209

Merged
hjmjohnson merged 223 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-mci
May 7, 2026
Merged

ENH: Ingest ITKMorphologicalContourInterpolation (stacked on #6208)#6209
hjmjohnson merged 223 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-mci

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented May 4, 2026

Copy link
Copy Markdown
Member

Ingests MorphologicalContourInterpolation from the standalone remote module KitwareMedical/ITKMorphologicalContourInterpolation into ITK at Modules/Filtering/MorphologicalContourInterpolation/. Stacked on PR #6208 (RLEImage ingest) — that PR provides MCI's TEST_DEPENDS RLEImage and the CTAD deduction guides that MCI's RLE-flavored test exercises.

Local validation: pre-commit run --all-files clean; MorphologicalContourInterpolationTestDriver builds; 368/368 MCI tests pass locally (plain + RLE-flavored, all algorithms B/C/T, all axis/label combinations).

BUG fix: thread_local SmartPointers removed (latest commit)

The upstream module used thread_local ITK SmartPointers in template functions as a per-thread filter cache. On MSVC, thread_local objects in template functions are destroyed when ITK's thread-pool workers exit at program shutdown — after ITK's reference-count infrastructure is partially torn down — causing SEGFAULTs. This manifested as 26 test failures on windows-2022 CI (GridSeg, GridSeg2, AccidentalMiddleSliceSeg, SevenLabels, FullEnd, ThreeAxisFourLabelConflict images). The fix replaces all 8 thread_local SmartPointer declarations with ordinary local variables; the per-thread caching is unnecessary under ITKv5's threading model and the allocation overhead is negligible.

What this provides

MorphologicalContourInterpolation provides filters that interpolate manually-segmented anatomical contours through volumetric label data using morphological reconstruction. Given a sparse set of 2D contours on arbitrary slices, the filter reconstructs a complete 3D label mask. The flagship class is itk::MorphologicalContourInterpolator.

Path: Modules/Filtering/MorphologicalContourInterpolation/ (alongside the existing Modules/Filtering/RLEImage/ from the stacked PR #6208).

Pipeline metrics
Metric Value
Upstream commits → rewritten 327 → 225
Upstream merges → preserved 77 → 46 (no linearization)
Blobs scanned in sanitize 1765 (cxx=266, py=0, cmake=63, text=369, skip=1067)
Blobs reformatted 317 (clang-format / gersemi / trailing-ws / EOF)
Commit subjects auto-prefixed 144
Subjects truncated to 78 chars 14
Blank lines inserted 131
Subtree merges visible after rebase 1

The relatively high commit count comes from MCI's long history (327 upstream commits since 2016) — reflects the maturity of this module.

Dependencies

EXCLUDE_FROM_DEFAULT is set on the module so it does not auto-build unless Module_MorphologicalContourInterpolation:BOOL=ON is passed.

Stack relationship to PR #6208

This PR stacks on PR #6208 (RLEImage ingest) for two reasons:

  1. TEST_DEPENDS RLEImage — MCI's itkMorphologicalContourInterpolationTestWithRLEImage.cxx instantiates the morphological filter against RLEImage types. Without RLEImage as an in-tree module, the test directory is not configured.

  2. CTAD deduction guides — when MCI's test code includes itkImageFileWriter.h (which transitively pulls in itkImageAlgorithm.hxx) and instantiates the writer's templates against an RLEImage, ITK core's CTAD-using calls (e.g. ImageRegionConstIterator it(inImage, inRegion);) need explicit deduction guides on the ImageRegionConstIterator<RLEImage<...>> partial specialization. Those guides were added in 8dcb1e8fe3 on PR ENH: Ingest ITKRLEImage into Modules/Filtering #6208. Without them, MCI's RLEImage-flavored test fails to compile.

If reviewers prefer to land MCI without the RLEImage dependency in this cycle, the alternative is to disable the itkMorphologicalContourInterpolationTestWithRLEImage.cxx source from test/CMakeLists.txt until RLEImage lands. The plain (non-RLE) MCI test suite does not need PR #6208.

Follow-up commits in this PR

After the merge commit (ENH: Ingest ITKMorphologicalContourInterpolation into Modules/Filtering):

  • DOC: Add Modules/Filtering/MorphologicalContourInterpolation/README.md pointing at the archived upstream.
  • COMP: Remove Modules/Remote/MorphologicalContourInterpolation.remote.cmake (now in-tree).
  • ENH: Enable Module_MorphologicalContourInterpolation:BOOL=ON in pyproject.toml configure-ci.
  • BUG: Replace thread_local SmartPointers with local variables (fixes 26 Windows CI SEGFAULTs).
Phase B (upstream archival) — deferred

Per the v4 design (PR #6204), the upstream archival step is intentionally separated. After this PR merges into ITK main, run:

~/src/ITK/Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh \
    MorphologicalContourInterpolation --itk-pr-number <THIS-PR>

Note the upstream is KitwareMedical/ITKMorphologicalContourInterpolation (not the InsightSoftwareConsortium org); confirm push permission before flipping archived=true.

AI assistance
  • Tool: Claude Code (claude-sonnet-4-6)
  • Role: root-cause analysis of 26 Windows CI SEGFAULTs and authoring the thread_local fix
  • Contribution: identified that thread_local ITK SmartPointers in template functions crash on MSVC at thread-pool shutdown by inspecting CI logs, tracing the thread_local introduction through git history to commit 017a693c5d (ITKv5 threading model switch), and confirming the fix compiles and all 368 MCI tests pass locally
  • All code was reviewed and tested locally before committing

thewtex and others added 30 commits April 24, 2015 14:24
ENH: adding the rest of test data from Paul
@hjmjohnson

Copy link
Copy Markdown
Member Author

Closing temporarily — this PR is stacked on #6208 (RLEImage ingest) and will be reopened (or re-submitted) once #6208 lands on main. MCI's tests use itk::RLEImage so the dependency must merge first.

The 11 missing CIDs are now published (ITKTestingData PR #47, merged), and the rerun of "Populate shared ExternalData cache" went green at ok=2457 fail=0. No work is lost; this is purely a queue-management close.

@dzenanz

dzenanz commented May 5, 2026

Copy link
Copy Markdown
Member

PR generally looks good. Modules/Filtering/MorphologicalContourInterpolation/CMakeLists.txt needs cleanup, like many recent ingestions.

@hjmjohnson

Copy link
Copy Markdown
Member Author

Rebased on upstream/main (now that #6208 has merged) and addressed @dzenanzs CMakeLists cleanup comment in c38495c9ea (drop cmake_minimum_required, the CXX_STANDARD policing block, and the if(NOT ITK_SOURCE_DIR) standalone-build wrapper). The v4 ingest pipeline (#6204) strips this boilerplate automatically going forward.

hjmjohnson and others added 5 commits May 6, 2026 14:21
Brings MorphologicalContourInterpolation from a configure-time remote fetch into the ITK
source tree at Modules/Filtering/MorphologicalContourInterpolation/ using the v4 ingestion
pipeline (whitelist filter-repo + per-commit clang-format + black +
commit-prefix sanitization).

Upstream repo:  https://github.com/KitwareMedical/ITKMorphologicalContourInterpolation.git
Upstream tip:   a0ed9f7fa58fc09270bb8f2c49a68243acfedd46
Ingest date:    2026-05-04
Whitelist:      MorphologicalContourInterpolation.list

Per-commit transforms applied across all 217 commits:
  - filter-repo --paths-from-file (whitelist)
  - filter-repo --to-subdirectory-filter Modules/Filtering/MorphologicalContourInterpolation
  - clang-format -style=file (ITK main's .clang-format) for *.cxx/.h/.hxx/...
  - black for *.py
  - heuristic ITK prefix added to commit subjects without one

Merge topology preserved: 77 -> 46 merge(s).

Primary author: Dženan Zukić <dzenan.zukic@kitware.com>

Co-authored-by: Adrien Boucaud <adrien.bk@free.fr>
Co-authored-by: Dzenan Zukic <dzenan.zukic@kitware.com>
Co-authored-by: Dženan Zukić <dzenanz@gmail.com>
Co-authored-by: Hans J. Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans.j.johnson@gmail.com>
Co-authored-by: Jon Haitz Legarreta <jhlegarreta@vicomtech.org>
Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com>
Co-authored-by: Mathew J. Seng <mathewseng@gmail.com>
Co-authored-by: Mathew Seng <mathewseng@gmail.com>
Co-authored-by: Matt McCormick <matt.mccormick@kitware.com>
Co-authored-by: Matt McCormick <matt@mmmccormick.com>
Co-authored-by: Stephen Aylward <stephen.aylward@kitware.com>
Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
Co-authored-by: vicory <jared.vicory@kitware.com>
…akeLists

Removes cmake_minimum_required, CXX_STANDARD policing, and the
standalone-build wrapper that the in-tree module never uses.
@hjmjohnson
hjmjohnson marked this pull request as ready for review May 6, 2026 19:58
@greptile-apps

greptile-apps Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (421 files found, 100 file limit)

thread_local ITK SmartPointers in template functions crash on MSVC at
program shutdown: ITK thread-pool workers destroy their thread_local
objects after ITK's reference-count infrastructure is partially torn
down.  The per-thread caching is unnecessary given ITKv5's threading
model; local variables are correct and the allocation overhead is
negligible relative to the filter computations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Enhancement Improvement of existing methods or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants