InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
ENH: Formalize highdicom fixtures as a GDCM/DCMTK/pydicom cross-validation testbed
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
Proposal to formalize the `highdicom` (ImagingDataCommons) DICOM test fixtures as a cross-validation testbed for ITK's DICOM readers (`GDCMImageIO`, `DCMTKImageIO`), decoding against `pydicom` as an independent ground truth. This grew out of triaging #6639 (single-bit decode fix) and directly found a second, separate bug (#6641, SEG-object single-bit decode) that neither project's own test suite had caught — real evidence this kind of cross-project cross-validation surfaces bugs standard same-codebase testing misses.
A working prototype already exists at `.devlocal/pr6639-triage/highdicom-crossval/` (not committed — local scratch) and a `ITK_USE_HIGHDICOM_TEST_DATA` CMake option has been added (default `OFF`, hard-errors unless `Module_ITKIODCMTK`, `Module_ITKIOGDCM`, and `ITK_WRAP_PYTHON` are all `ON`) as the gate this work would eventually sit behind.
Background / case study
While fixing #6629 → #6638 (revert) → #6639 (forward-fix) for GDCM's single-bit DICOM pixel decode, I built a small Python harness comparing `itk.GDCMImageIO`/`itk.DCMTKImageIO` pixel output against `pydicom.pixel_array` as ground truth, using 25 real DICOM fixtures vendored from `ImagingDataCommons/highdicom`'s `data/test_files/` (MIT licensed, ~556 KB total). The harness correctly distinguishes real decode bugs from expected semantic differences (Modality LUT rescale, MONOCHROME1→MONOCHROME2 inversion) before reporting a mismatch.
Result: confirmed #6639's fix is correct and does not regress any of these fixtures, **and** found a second, unrelated bug — `GDCMImageIO` returns unscaled 0–255 byte-range garbage (not unpacked binary 0/1) for single-bit DICOM **Segmentation Storage** (SEG) objects, filed as #6641. Two more SEG fixtures fail outright, and four more throw a `cosines.size() == 6` internal error — likely a related gap in per-frame orientation lookup for multi-frame SEG objects.
Impediments to formalizing (from initial assessment)
1. **The known bug (#6641) needs a decision on fixture handling** — either fix it first, or mark those specific fixtures as expected-fail, before wiring this in as a hard CTest gate.
2. **New Python dependency** — `pydicom` is not currently a dependency of any existing ITK Python test (which use only `itk` + numpy). Needs its own optional-dependency detection.
3. **DCMTK comparison requires Python wrapping of `ITKIODCMTK`**, which is `EXCLUDE_FROM_DEFAULT` and not wrapped in any current CI configuration — needs a dedicated build variant to exercise.
4. **Fixture provenance audit is unfinished** — need to confirm none of highdicom's SEG/SM test fixtures embed real (even anonymized) patient-derived pixel data with its own usage terms, before permanently vendoring into `ITKTestingData`.
5. **Test infra choice** — likely NOT a CTest-gated default (small, opt-in, Python-dependent), more likely a `pixi run`-style manual/CI-opt-in suite under `Testing/Python/`, gated by the new `ITK_USE_HIGHDICOM_TEST_DATA` CMake option.
### Proposed scope of work
- [ ] Root-cause and fix #6641 (or explicitly scope it out of this issue if it's larger than expected)
- [ ] License/provenance audit of all vendored highdicom fixtures intended for `ITKTestingData`
- [ ] Decide fixture storage: content-linked `ITKTestingData` (`.cid`) vs. small enough to commit directly
- [ ] Wire a `pydicom`-dependent Python test driver under `Testing/Python/`, gated by `ITK_USE_HIGHDICOM_TEST_DATA`
- [ ] CI variant enabling `Module_ITKIODCMTK` + `ITK_WRAP_PYTHON` + `ITK_USE_HIGHDICOM_TEST_DATA` to actually run this in CI (optional/nightly, not every PR)
- [ ] Investigate the `cosines.size() == 6` multi-frame SEG orientation failures — same root cause as #6641 or separate?
Contributor guide
Research direction
Start by reviewing the existing ITK_USE_HIGHDICOM_TEST_DATA option in CMakeLists.txt and the local prototype under .devlocal/pr6639-triage/highdicom-crossval/. Then inspect Testing/Python/ and related ITKTestingData conventions before resolving the #6641, dependency, provenance, and CI questions. Done means the opt-in cross-validation suite, fixture handling, and CI variant are defined and functioning.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, python
- Domain
- build-system, testing
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100