Skip to content

BUG: Backport zero-check guard in CumulativeGaussianCostFunction#6067

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-cumulative-gaussian
Apr 16, 2026
Merged

BUG: Backport zero-check guard in CumulativeGaussianCostFunction#6067
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-cumulative-gaussian

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Adapted from d7845fe on main. Adds numberOfElements == 0 check before the size-mismatch check in CalculateFitError to guard against division-by-zero.

On main this was a reorder of an existing compound condition; on release-5.4 the zero check was entirely absent.

Backport for #6051 (Tier 1).

Guard the division by numberOfElements on line 55 against
division-by-zero when m_OriginalDataArray is empty. The existing
size-mismatch check did not catch the zero case.

Cherry-pick of d7845fe from main, adapted for release-5.4
where the zero check was entirely absent (main had it in the
wrong order).
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances area:Numerics Issues affecting the Numerics module labels Apr 15, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

This backport adds a numberOfElements == 0 early-return guard in CalculateFitError (line 46) via short-circuit || evaluation, preventing a division-by-zero crash that was entirely absent in the release-5.4 branch. The fix is correctly ordered and the guard logic is sound.

Confidence Score: 5/5

Safe to merge; the zero-check guard is correct and no new defects are introduced.

The primary fix is correct — numberOfElements == 0 is checked before the mismatch check using short-circuit evaluation, fully preventing division-by-zero. The only finding is a pre-existing integer division issue (1 / numberOfElements) that is unrelated to this PR and does not block merge.

No files require special attention.

Important Files Changed

Filename Overview
Modules/Numerics/Optimizers/src/itkCumulativeGaussianCostFunction.cxx Adds numberOfElements == 0 guard via short-circuit `

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CalculateFitError(setTestArray)"] --> B["numberOfElements = m_OriginalDataArray.GetNumberOfElements()"]
    B --> C{"numberOfElements == 0?"}
    C -- "Yes (NEW guard)" --> D["return 1"]
    C -- "No" --> E{"numberOfElements != setTestArray->size?"}
    E -- "Yes" --> D
    E -- "No" --> F["Sum squared differences in loop"]
    F --> G["return sqrt((1/numberOfElements) * fitError)"]
    style C fill:#90ee90,stroke:#228B22
    style D fill:#ffcccc
Loading

Comments Outside Diff (1)

  1. Modules/Numerics/Optimizers/src/itkCumulativeGaussianCostFunction.cxx, line 55 (link)

    P2 Pre-existing integer division yields incorrect RMSE

    1 / numberOfElements performs integer division because both operands are integer types (int literal and unsigned int). For any numberOfElements > 1, this evaluates to 0, making sqrt(0 * fitError) == 0.0 — the RMSE is silently wrong for all non-trivial arrays. The PR correctly guards against numberOfElements == 0, but since this function is being modified, this is a good opportunity to fix the formula.

Reviews (1): Last reviewed commit: "BUG: Add zero-check guard in CumulativeG..." | Re-trigger Greptile

Copy link
Copy Markdown
Member Author

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

greptile comments need to ensure that upstream/main has already fixed this, if not fix upstream/main and PR.

This is a backport from main -> release-5.4.

Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hjmjohnson thanks!

@thewtex thewtex added this to the ITK 5.4.6 milestone Apr 15, 2026
@hjmjohnson hjmjohnson merged commit 52b06eb into InsightSoftwareConsortium:release-5.4 Apr 16, 2026
19 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Numerics Issues affecting the Numerics module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants