Skip to content

BUG/REV: Compute Feature Neighbors#1569

Merged
nyoungbq merged 16 commits into
BlueQuartzSoftware:developfrom
nyoungbq:rev/find_feature_neighbors
Mar 25, 2026
Merged

BUG/REV: Compute Feature Neighbors#1569
nyoungbq merged 16 commits into
BlueQuartzSoftware:developfrom
nyoungbq:rev/find_feature_neighbors

Conversation

@nyoungbq
Copy link
Copy Markdown
Contributor

@nyoungbq nyoungbq commented Mar 25, 2026

  • Patched Major bug in calculation of Shared Surface Area List (Present in 6.5)
  • Added 32 new test cases covering 3D/2D/1D/0D
  • Hand validated output for each new test case
  • Algorithm restructuring to reduce time complexity and branching in tight loops
  • deepest nested loop reduction for non 3D cases
  • extensive conversion of runtime branching to compile time constexpr
  • assorted optimizations following profiling
  • code documentation

3D Hand Validation attached
compute_feature_neighbors_3D_ test.txt

@nyoungbq nyoungbq requested a review from JDuffeyBQ March 25, 2026 13:35
Copy link
Copy Markdown
Collaborator

@JDuffeyBQ JDuffeyBQ left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of minor suggestions.

@nyoungbq nyoungbq requested a review from JDuffeyBQ March 25, 2026 15:53
@nyoungbq nyoungbq force-pushed the rev/find_feature_neighbors branch from e37959a to 8d35079 Compare March 25, 2026 16:24
@nyoungbq nyoungbq merged commit 9e61b41 into BlueQuartzSoftware:develop Mar 25, 2026
6 checks passed
@nyoungbq nyoungbq deleted the rev/find_feature_neighbors branch March 25, 2026 18:37
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Mar 30, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Mar 30, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Apr 1, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Apr 2, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Apr 2, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Apr 2, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
joeykleingers added a commit to joeykleingers/simplnx that referenced this pull request Apr 3, 2026
…riants

Integrate Nathan Young's ComputeFeatureNeighbors rewrite (PR BlueQuartzSoftware#1569) into
the OOC dispatch architecture:

Direct variant: full port of Nathan's algorithm with compile-time
ImageDimensionState specialization, two-stage boundary/internal cell
processing, and per-face surface area accumulation. Handles 0D through
3D geometries via constexpr template dispatch.

Scanline variant: updated to use map-based per-face surface area
accumulation (fixes DREAM3D 6.5 surface area calculation bug), correct
surface feature detection for all geometry dimensions (0D/1D/2D/3D),
and Path-based InputValues fields matching upstream.

Test file: added ForceOocAlgorithmGuard dual-path coverage to all 33
test cases (32 hand-validated + legacy SmallIn100).

Verified: 33/33 tests pass on both in-core (InCoreOnly) and OOC
(OocOnly) configurations.
imikejackson added a commit that referenced this pull request May 4, 2026
Batch D covers c-axis misalignments and neighbor-correlation
filters. All reports remain DRAFT pending developer review of the
tentative Algorithm Relationship and Oracle classifications.

* ComputeFeatureNeighborCAxisMisalignmentsFilter — DIVISOR BUG
  REPLICATED VERBATIM from sibling
  ComputeFeatureNeighborMisorientationsFilter:
  hexNeighborListSize is reassigned on line 111 of the algorithm,
  clobbering the hexNeighborListSize-- decrement on line 150 and
  producing wrong per-feature AvgCAxisMisalignments whenever any
  non-hex neighbor exists. Production-relevant because
  EBSD_Hexagonal_Data_Analysis.d3dpipeline ships with
  find_avg_misals: true. Existing test exemplar is hex-only so it
  cannot trigger the bug. PR #1467 was OEM-reviewed but preserved
  the bug because the review covered parameters, not divisor logic.
* ComputeFeatureNeighborsFilter — PR #1569 fixed an explicit "Major
  bug in calculation of Shared Surface Area List (Present in 6.5)";
  the shared 6_6_stats_test_v2.tar.gz SSA values are confirmed
  bad-from-legacy and are intentionally skipped in the
  Legacy:SmallIn100 test. PR #1590 fixed an in-house row-stride
  bug in the EmptyZ/EmptyY/EmptyX dispatchers that had been masked
  by all 5x5x1 fixtures having dims[0] == dims[1]. 32 hand-derived
  inline test cases now cover 0D/1D/2D/3D x empty-axis x optional
  flag combinations.
* BadDataNeighborOrientationCheckFilter — PR #1499 (REV) is the
  model V&V case in the audit: explicit iteration-guard bug fix
  paired with 28 algorithmic test cases (~1700 lines) using
  hand-derived expectedMask arrays as a de facto Class-1
  analytical oracle. PR #1590 made the 6-face neighbor logic
  2D-aware via NeighborUtilities.
* NeighborOrientationCorrelationFilter — currentLevel > Level
  strict inequality means Level=6 is a no-op (surprising
  semantics); only one end-to-end algorithmic test exists; PR
  #1472 introduced a float-to-double widening that may drift vs
  legacy.

Cross-cutting:
- The divisor-bug pattern is now confirmed as a TWO-FILTER copy-
  paste pattern, not isolated. Screen remaining "average across
  neighbors" filters for the same shape (NeighborList::size()
  reassigned inside the inner loop, clobbering an earlier --).
- Two AUDIT-CONFIRMED legacy 6.5 defects (PR #1569 SSA, PR #1499
  iteration guard) — first instances where legacy is provably
  wrong rather than SIMPLNX merely diverging.
- The model V&V pattern is hand-derived expected* arrays inline in
  test source. Filters with one happy-path exemplar test are
  measurably weaker.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants