Skip to content

BUG: Make FastMarching AllTargets stop condition count distinct targets#6658

Open
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:bug-fastmarching-alltargets-6575
Open

BUG: Make FastMarching AllTargets stop condition count distinct targets#6658
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:bug-fastmarching-alltargets-6575

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

Fixes the AllTargets stop condition in FastMarchingUpwindGradientImageFilter, which compared the reached-target count against the raw target-container size and so never triggered when the target list held duplicate indices. Addresses item B24 of #6575.

Root cause & fix

Each index is accepted (reached) at most once, so a target container with duplicate indices could never satisfy reachedCount == containerSize; AllTargets mode therefore never halted and the front marched the whole image.

The filter now computes the number of distinct target indices once in Initialize() (via std::set<IndexType>) and compares the reached count against that. The change is scoped strictly to the AllTargets branch of UpdateNeighbors; SomeTargets/OneTarget behavior is unchanged.

Test

Adds a GoogleTest with a duplicated target index. On the unfixed code GetTargetValue() stays 0 (target never registered / never halts as intended); after the fix it reports the correct arrival time. Fail-before/pass-after was verified locally. Self-contained synthetic image — no ITKTestingData fixture.

FastMarchingUpwindGradientImageFilter's AllTargets stop condition
compared the reached-target count against the raw target-container size.
Because each index is accepted (and so reached) at most once, a target
container with duplicate indices could never satisfy the equality, so
AllTargets mode never triggered and the front marched the whole image.

Compute the number of distinct target indices once in Initialize() and
compare against that. Adds a GoogleTest with a duplicated target index,
which leaves GetTargetValue() at 0 on the unfixed code (target never
registered) and reports the correct arrival time after the fix.

Issue InsightSoftwareConsortium#6575, item B24.
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module labels Jul 17, 2026
@hjmjohnson
hjmjohnson marked this pull request as ready for review July 17, 2026 18:43
@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes AllTargets stopping for duplicate fast-marching targets. The main changes are:

  • Counts distinct target indices during FastMarchingUpwindGradientImageFilter::Initialize().
  • Uses the distinct target count when deciding whether AllTargets has been reached.
  • Adds a GoogleTest case for duplicate target entries.
  • Registers the new UpwindGradient GoogleTest source in the FastMarching test driver.

Confidence Score: 5/5

Safe to merge with minimal risk.

The change is narrow, initializes the new count on each run, preserves OneTarget and SomeTargets behavior, and includes focused test coverage.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex ran the requested general contract validation.
  • T-Rex did not upload local artifact references with the verification run.

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Filtering/FastMarching/include/itkFastMarchingUpwindGradientImageFilter.h Adds private storage for the distinct target count used by AllTargets; no functional issues found.
Modules/Filtering/FastMarching/include/itkFastMarchingUpwindGradientImageFilter.hxx Computes distinct target indices during initialization and uses that count for AllTargets termination; no functional issues found.
Modules/Filtering/FastMarching/test/CMakeLists.txt Adds the new UpwindGradient GoogleTest source to the FastMarching GTest driver; no issues found.
Modules/Filtering/FastMarching/test/itkFastMarchingUpwindGradientImageFilterGTest.cxx Adds a synthetic duplicate-target regression test for AllTargets behavior; no issues found.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as Caller
participant Filter as FastMarchingUpwindGradientImageFilter
participant Targets as TargetPoints
participant Reached as ReachedTargetPoints
Caller->>Filter: SetTargetPoints(duplicates)
Caller->>Filter: SetTargetReachedModeToAllTargets()
Caller->>Filter: Update()
Filter->>Filter: Initialize()
Filter->>Targets: iterate target indices
Filter->>Filter: compute m_TotalDistinctTargets
loop accepted front indices
    Filter->>Targets: compare accepted index
    alt target index matched
        Filter->>Reached: InsertElement(distinct reached target)
        Filter->>Filter: compare Size() to m_TotalDistinctTargets
    end
end
Filter-->>Caller: TargetValue and output
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller as Caller
participant Filter as FastMarchingUpwindGradientImageFilter
participant Targets as TargetPoints
participant Reached as ReachedTargetPoints
Caller->>Filter: SetTargetPoints(duplicates)
Caller->>Filter: SetTargetReachedModeToAllTargets()
Caller->>Filter: Update()
Filter->>Filter: Initialize()
Filter->>Targets: iterate target indices
Filter->>Filter: compute m_TotalDistinctTargets
loop accepted front indices
    Filter->>Targets: compare accepted index
    alt target index matched
        Filter->>Reached: InsertElement(distinct reached target)
        Filter->>Filter: compare Size() to m_TotalDistinctTargets
    end
end
Filter-->>Caller: TargetValue and output
Loading

Reviews (1): Last reviewed commit: "BUG: Make AllTargets stop condition coun..." | Re-trigger Greptile

@hjmjohnson
hjmjohnson requested a review from dzenanz July 20, 2026 23:23
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:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant