Skip to content

ENH: Restore itkYvvBenchmark3D with ITKTestingData CID content-link#6265

Open
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:restore-itkYvvBenchmark3D
Open

ENH: Restore itkYvvBenchmark3D with ITKTestingData CID content-link#6265
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:restore-itkYvvBenchmark3D

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Restores the itkYvvBenchmark3D test that was dropped in commit c2940bf (part of PR #6243) because the input fixture 256x256x64.tif was not yet published. The fixture has since been published in ITKTestingData PR #49 (merged) under CID bafkreihivrgyltvbk7qsdgxvuknyikkfnxijm6wgya66zi5zzjmyiei3da, so the test can now resolve DATA{Input/256x256x64.tif} via ExternalData.

Test plan
  • Configured local build with Module_SmoothingRecursiveYvvGaussianFilter=ON (the module is EXCLUDE_FROM_DEFAULT).
  • Built SmoothingRecursiveYvvGaussianFilterTestDriver — clean link.
  • Triggered ExternalData fetch — blob (4.0 MiB) downloaded and symlinked to ITK_DATA_CACHE/CID/bafkreihivr….
  • Ran ctest -R '^itkYvvBenchmark3D$'passed in 0.21s.
  • pre-commit run --all-files clean on post-commit tree.
What this PR changes

Two-file change (+12 / -0):

  • Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/test/Input/256x256x64.tif.cid — new content-link file with the ITKTestingData CID.
  • Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/test/CMakeLists.txt — restores the itk_add_test(NAME itkYvvBenchmark3D …) block, invoking itkYvvBenchmark with size=3, sigma=12.0, iterations=2 on the 256×256×64 input.

The drop in c2940bf retained 3D coverage via itkYvvWhiteImageTest3D and the GPU 3D filter test; this PR adds back the benchmark variant that exercises the recursive Yvv-Gaussian filter on a real medical-imaging-shaped volume.

Reverses the partial drop in c2940bf ("BUG: Drop itkYvvBenchmark3D
— input fixture 256x256x64.tif not available"). The fixture is now
published in ITKTestingData as CID
bafkreihivrgyltvbk7qsdgxvuknyikkfnxijm6wgya66zi5zzjmyiei3da
(ITKTestingData PR InsightSoftwareConsortium#49, merged), so the 3D benchmark can resolve
DATA{Input/256x256x64.tif} via ExternalData.

Restores:
  * Input/256x256x64.tif.cid — content-link to the published fixture.
  * itk_add_test(NAME itkYvvBenchmark3D ...) in test/CMakeLists.txt,
    invoking itkYvvBenchmark with size=3, sigma=12.0, iterations=2 on
    the 256x256x64.tif input (4.0 MiB, sha256
    e8ac4d85cea157e1219af5a29b8429456dd0967ac6c03deca3b9ca5984111b18).
@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Filtering Issues affecting the Filtering module labels May 13, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review May 13, 2026 22:44
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

Restores the itkYvvBenchmark3D test and adds the corresponding 256x256x64.tif.cid content-link so CMake ExternalData can fetch the 4 MiB fixture from the ITKTestingData CID store.

  • test/Input/256x256x64.tif.cid — new CID content-link (bafkreihivr…) pointing to the now-published ITKTestingData fixture.
  • test/CMakeLists.txt — restores itk_add_test(NAME itkYvvBenchmark3D …) with itkYvvBenchmark, size=3, sigma=12.0, iterations=2, which matches the parameters used by the benchmark before it was dropped.

Prose-budget violation (commit body + PR visible summary). The commit body's "Restores:" bullet section restates what the diff already shows (forbidden per prose-budget.md — "Explanations of code that the diff already shows") and the body is 13 lines, exceeding the 12-line cap. The PR visible summary (~385 chars) also exceeds the 250-char cap. Both should be trimmed before merge.

Confidence Score: 4/5

The two-file change correctly restores the benchmark test and its content-link; the only issue is that the commit body and PR visible summary are longer than the repo's hard caps allow.

Both changed files are mechanically correct — the CID is well-formed and the CMake test block matches the pre-drop version and the 2D sibling test. The commit body's "Restores:" bullet section duplicates information that the diff already conveys and pushes the body to 13 lines (repo cap is 12); the visible PR summary is ~385 chars against a 250-char cap. The prose-budget doc marks these as review-blocking defects, so the commit needs a reword before this is clean to merge.

No files require special attention for correctness; the commit message itself needs to be trimmed.

Important Files Changed

Filename Overview
Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/test/CMakeLists.txt Restores the itkYvvBenchmark3D test block; parameters and DATA{} reference are consistent with the 2D benchmark and the new .cid fixture.
Modules/Filtering/SmoothingRecursiveYvvGaussianFilter/test/Input/256x256x64.tif.cid New CID content-link file; CIDv1 format matches all other .cid files in the repo, trailing newline present.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["cmake configure\n(Module_SmoothingRecursiveYvvGaussianFilter=ON)"] --> B["ExternalData sees\nDATA{Input/256x256x64.tif}"]
    B --> C["Reads 256x256x64.tif.cid\nbafkreihivr…"]
    C --> D{"Blob in\nlocal cache?"}
    D -- Yes --> E["Symlink object\n→ build tree"]
    D -- No --> F["Fetch from\nITKTestingData CID store"]
    F --> E
    E --> G["ctest -R itkYvvBenchmark3D\nitkYvvBenchmark size=3 sigma=12.0 iters=2"]
    G --> H["Test passes"]
Loading

Reviews (1): Last reviewed commit: "ENH: Restore itkYvvBenchmark3D with ITKT..." | Re-trigger Greptile

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:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant