Skip to content

VV: Filter V&V Action List#1611

Open
imikejackson wants to merge 10 commits into
developfrom
topic/vv_retroactive_audit
Open

VV: Filter V&V Action List#1611
imikejackson wants to merge 10 commits into
developfrom
topic/vv_retroactive_audit

Conversation

@imikejackson
Copy link
Copy Markdown
Contributor

No description provided.


## Thread 1 — Oracle versioning (preventing "oracle drift")

Each oracle class has a different fragility profile:
Copy link
Copy Markdown
Contributor

@nyoungbq nyoungbq May 1, 2026

Choose a reason for hiding this comment

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

Instead of oracle, I think long-term test fidelity profile clarifies the intention of the label, although I do recognize it is wordier

Edit: rename suggestion from long-term data fidelity profile to long-term test fidelity profile


## Provenance required in archive ReadMe

- **Class 2:** library + exact version + seed (if any) + script filename. Optional: hash of script's output for drift detection.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand what it is referencing the table with the class 2 but I really feel like it should inline the name of what it is referring to so it can be read easily. Right now it reads like a bad variable name for a code equivalent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this whole page is basically breaking down how to do black-box testing methodology as far as I can tell, but the naming scheme is really throwing me off readability wise

### 2. Deviation Taxonomy

When outputs differ, what are the legal outcomes? I'd expect four:
(i) legacy bug → patch legacy;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After meeting I think this should be changed to just documenting it in the known issues wont fix page


## Archiving Everything When Finished

- As a standard operating procedure, I am uploading all data files and pipelines (for both 6.5.171 and NX) up to OneDrive into a folder specific to that filter verification. This folder can contain even more data and notes that you may have used to verify the filter.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer a new public repo for a website that could host all this information or merging it into our docs and building against it with a separate repo for data that can be linked back to instead of OneDrive, but that's up to you

Adds the per-filter retroactive Verification & Validation reports
generated as part of the MTR (SBIR) audit. Each report inventories the
filter's source files, walks the full git history since 2025-10-01,
classifies material vs. broad-refactor PRs, captures test coverage and
exemplar-archive references, and proposes a tentative Algorithm
Relationship + Oracle class. All reports are DRAFT — developers must
confirm or correct the tentative classifications.

Template-of-record:

* CAxisSegmentFeaturesFilter — approved sample format

Batch A (counting/centroid/phase/copy/sample, all SimplnxCore):

* ComputeFeatureCentroidsFilter — flags Kahan compensated summation
  and untested IsPeriodic=true branch as deviation candidates
* ComputeFeatureSizesFilter — PR #1540 dominates the V&V history
  (Kahan summation, int32 overflow guard, degenerate-dim preflight,
  +639 lines of hand-derived analytical-oracle test code)
* ComputeFeaturePhasesFilter — algorithm body untouched in window;
  existing test is implicitly an agreement-with-legacy check
* CopyFeatureArrayToElementArrayFilter — surfaces two API
  deviations vs. SIMPL (single-to-multi feature input,
  CreatedArrayName-to-CreatedArraySuffix) and a weak existing test
* IdentifySampleFilter — slice-by-slice mode, 2D dispatch, and
  uint8 mask are net-new SIMPLNX capabilities (added-capability
  deviations, not behavioral)

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Batch B covers rotations, orientation conversion, IPF coloring, and
threshold logic. All reports remain DRAFT pending developer review of
the tentative Algorithm Relationship and Oracle classifications.

* RotateSampleRefFrameFilter (SimplnxCore) — flags the stale "only
  90/180 deg verified" doc warning (45 deg tests were added in
  PR #1465); both v2 and v3 test archives still listed in
  CMakeLists; rotation math lives in shared ImageRotationUtilities
* RotateEulerRefFrameFilter (OrientationAnalysis) — PR #1472
  silently widened the per-voxel arithmetic from float to double
  precision (OrientationF -> *DType, Matrix3fR -> Matrix3dR);
  strongest deviation candidate vs legacy SIMPL
* ComputeIPFColorsFilter (OrientationAnalysis) — PR #1472 (EbsdLib
  2.0.0 bump) flagged as prime RGB-drift suspect; coverage is
  cubic-high + [001] only; existing diagnostic block in test asserts
  nothing
* ConvertOrientationsFilter (OrientationAnalysis) — PR #1472 was a
  wholesale rewrite (~+387/-1190 net) of the dispatch and math
  layer, not a rename; PR #1438 hides a real algorithmic fix
  (getPositiveOrientation + zero-init); no exemplar archive;
  Stereographic excluded from cross-product; no round-trip test
* MultiThresholdObjectsFilter (SimplnxCore) — suspected real bug:
  std::reverse is used in the replaceInput && inverse branch where
  boolean NOT appears to be intended; non-linear extraction story
  (#1301 stub -> #1521 not_used -> #1544 populated); dual-source
  SIMPL port consolidating MultiThresholdObjects v1 + v2

Cross-cutting: PR #1472 (EbsdLib 2.0.0) is materially significant
for any filter that delegates math to EbsdLib and should be promoted
out of the pruned list for those filters going forward.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Batch C covers orientation/c-axis math filters in OrientationAnalysis.
All reports remain DRAFT pending developer review of the tentative
Algorithm Relationship and Oracle classifications.

* ComputeAvgOrientationsFilter — PR #1577 added vMF and Watson
  EM-based averaging methods; the filter now hosts three distinct
  algorithms under one UUID. v2 exemplars for the new methods are
  likely self-referential (added in the same PR). PR #1472
  downgraded to pruned after scoped diff inspection (API rename
  only for this filter).
* ComputeAvgCAxesFilter — PR #1438 silently changed three things:
  float-to-double accumulation, antipodal-flip behavior via counter
  reorder, and error-code renumbering. Exemplar 7_2_AvgCAxis.tar.gz
  is likely circular (regenerated post-fix). Mixed-phase warning
  path and counter==0 rescue path uncovered.
* ComputeCAxisLocationsFilter — PR #1472 silently rewrote the
  qu-to-om path through a different EbsdLib API
  (Quaternion::toOrientationMatrix vs explicit qu2om), proposed as
  Deviation D4 pending comparison. PR #1582 cancel path returns
  success while leaving output partially zero-filled (D3 internal
  correctness issue).
* ComputeFeatureReferenceCAxisMisorientationsFilter — PR #1438 has
  an explicit REV: review tag and StdDev double-precision fix that
  per the PR description brings output into agreement with
  DREAM3D > 6.5.172, but the change is not regression-locked by
  any new test in the same PR.
* ComputeFeatureNeighborMisorientationsFilter — suspected
  port-fidelity bug: tempMisoList = featureNeighborList.size() is
  reassigned inside the inner j-loop, so the divisor reflects only
  the last neighbor's match state instead of a true matched-phase
  count. The ComputeAvgMisors=true test is an [.][UNIMPLEMENTED]
  stub, so the entire average-misorientation control flow has zero
  CI coverage.

Cross-cutting: PR #1438 is the largest hidden-deviation hotspot
surfaced in the audit so far — its "ENH" label is misleading; the
PR was actually a correctness sweep with multiple silent
algorithmic shifts. Multiple exemplar archives in this batch
appear to be circular (regenerated from post-fix SIMPLNX output),
so existing tests cannot prove legacy conformance even where they
appear to.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
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>
Batch E covers I/O and remaining microtexture / cleanup filters.
This is the final batch of the 22-filter Tier 1 retroactive audit.
All reports remain DRAFT pending developer review of the tentative
Algorithm Relationship and Oracle classifications.

* FillBadDataFilter — Algorithm Relationship: REWRITE. PR #1515
  was an AI-generated four-phase chunk-aware CCL+UnionFind
  rewrite (+1139/-194 lines) that has never been formally
  reviewed; the only legacy-conformance claim is in the PR
  description. Two real bugs surfaced: (1) static_cast<int32>(size)
  < threshold at line 476 wraps for components > 2.1B voxels (D4);
  (2) potential infinite loop in Phase 4 — while(count != 0) has
  no no-progress termination (D3). storeAsNewPhase assigns the
  same phase index to all large defect regions (D1). Archive
  6_5_fill_bad_data.tar.gz violates the project's own naming
  guidance — the data is post-#1515 NX-generated, not legacy.
* ReadAngDataFilter — PR #1462 deliberately replaced the legacy
  6.6 oracle with an NX-self-generated exemplar; the V&V oracle
  effectively shifted to Class 5 regression / golden-master.
  PR #1588 added 6.4 + 6.5 conversion JSONs but no TEST_CASE
  to exercise them. Phase index < 1 is silently clamped to 1,
  losing EDAX bad-phase signal (D3). PR #1472 correctly
  downgraded after diff inspection (pure namespace renames).
* ITKImageWriterFilter — PR #1555 silently broke OOC support via
  dynamic_cast<DataStore<T>> with no preflight gate. WriteAs2DStack
  has an off-by-one loop bound that appears to be dead code. Test
  only verifies output-file existence and count, never pixel
  content. No Algorithms/ extraction — all logic in the filter
  .cpp anonymous namespace.
* WritePoleFigureFilter — PR #1587 is unusually material: 391
  lines of layout/rendering moved into EbsdLib 2.4.0 and the
  exemplar archive bumped v4 → v5. Tests provide no signal about
  pre-#1587 vs post-#1587 numerical drift. Two compounding
  EbsdLib bumps (#1472 → 2.0.0 and #1587 → 2.4.0) make the
  legacy comparison especially important.
* ReplaceElementAttributesWithNeighborValuesFilter — Two real
  bugs: (1) bestNeighbor vector allocated once before
  while(keepGoing) and never reset between iterations, so a
  voxel replaced in iteration N can be silently re-replaced in
  N+1 from a stale entry (D1); (2) float32 best is used
  regardless of templated type T, truncating int64/uint64/
  float64 values (D2). Existing test loads the same .dream3d
  file as both input and exemplar — may not actually exercise
  replacement. Significant unmerged work exists on remote
  branches joey/ooc-filter-optimizations and joey/worktree-
  ReplaceElementAttrsWithNeighborValuesValidation.
* ComputeGroupingDensityFilter — surprise: despite being
  brand-new in the audit window (PR #1548), this is a Port,
  not a New filter (legacy DREAM3D FindGroupingDensity renamed
  per the Find* → Compute* convention), so legacy comparison
  applies. Test tag bug: 7 of 8 TEST_CASEs tagged
  [SimplnxReview] instead of [SimplnxCore], suggesting the
  filter was prototyped in SimplnxReview before being moved.
  Existing tests have hand-derived expected constants — Class 1
  oracle gold standard.

Cross-cutting Batch E findings:
- Five new real-bug candidates surfaced (FillBadData int32
  overflow, FillBadData potential infinite loop, ITKImageWriter
  OOC break, ReplaceElementAttributes bestNeighbor stale-state,
  ReplaceElementAttributes float32-truncation). Add to the audit
  bug log alongside the prior 3 (MultiThresholdObjects
  std::reverse + two-filter divisor pattern).
- AI-generated rewrite (PR #1515) without formal review is a
  V&V red flag that the policy doc should explicitly call out
  as requiring extra scrutiny (algorithm review + independent
  oracle, not just "passes existing tests").
- Multiple filters have deliberately replaced their legacy
  oracle with an NX-self-generated exemplar (FillBadData
  6_5_fill_bad_data.tar.gz, ReadAng read_ang_test.tar.gz,
  WritePoleFigure v5 archive, AvgCAxes 7_2_AvgCAxis.tar.gz),
  each shifting the de-facto oracle to Class 5 regression /
  golden-master. The policy should explicitly require an
  Oracle Provenance ReadMe entry for any such archive.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Summary entry-point for the 22-filter Tier 1 retroactive audit.
Provides:

* At-a-glance metrics (filter counts, bug counts, oracle-class
  distributions)
* Master table by batch with one row per filter, linking to each
  per-filter report (tentative Algorithm Relationship, tentative
  Oracle class, material PR count, bug flag)
* Suspected-real-bugs table — 8 distinct bugs across 5 filters,
  with severity and production-relevance flags. Calls out the
  ComputeFeatureNeighborMisorientations / ComputeFeatureNeighbor-
  CAxisMisalignments divisor pattern as confirmed copy-paste.
* Seven cross-cutting findings drawn from the audit (PR #1438
  hidden-deviation hotspot, PR #1472 heterogeneity, the model V&V
  pattern of inline hand-derived expected arrays, the circular-
  oracle pattern across 5 filters, two audit-confirmed legacy 6.5
  defects, shared-archive contagion via 6_6_stats_test_v2.tar.gz,
  AI-generated rewrite as a V&V red flag).
* Next steps — bug triage, skill design, SimplnxReview-plugin
  reports, policy amendments.

All per-filter reports remain DRAFT; this index is also DRAFT and
will be updated as developers confirm or correct the tentative
classifications.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
@imikejackson imikejackson force-pushed the topic/vv_retroactive_audit branch from ae2e8d0 to 8071cad Compare May 4, 2026 14:57
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
- **Files in this filter:** filter (.hpp +126, .cpp +278), algorithm (.hpp +52, .cpp +219), doc (.md +90), three doc images (Algorithm.png, FeatureIds.png, ParentIds.png), legacy UUID map (+3 lines), plugin CMakeLists (+2), test CMakeLists (+2), unit test (.cpp +431).
- **Diff size:** 12 files, +1203 lines, –0.
- **Change nature:** **New filter / Port.** Initial introduction of `ComputeGroupingDensityFilter` plus its algorithm, documentation (with worked example and three illustrative images), `FromSIMPLJson()` conversion path, legacy UUID map entry, and a 431-line test file containing one exemplar-based test, four execution tests covering all four template specializations of `FindDensityGrouping<UseNonContiguousNeighbors, FindCheckedFeatures>`, and three preflight error tests. The filter is implemented as the standard Filter+Algorithm pair per the project convention and uses `MessageHelper`/`ThrottledMessenger` for progress reporting and a `m_ShouldCancel` check inside the parent loop.
- **V&V content:** **High** for a single-PR introduction.
Copy link
Copy Markdown
Contributor

@nyoungbq nyoungbq May 6, 2026

Choose a reason for hiding this comment

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

This needs clarification, does high a measure of work to do, or completeness of existing information, and what is the scale for said "measurement"


- **Files in this filter:** filter (.hpp +126, .cpp +278), algorithm (.hpp +52, .cpp +219), doc (.md +90), three doc images (Algorithm.png, FeatureIds.png, ParentIds.png), legacy UUID map (+3 lines), plugin CMakeLists (+2), test CMakeLists (+2), unit test (.cpp +431).
- **Diff size:** 12 files, +1203 lines, –0.
- **Change nature:** **New filter / Port.** Initial introduction of `ComputeGroupingDensityFilter` plus its algorithm, documentation (with worked example and three illustrative images), `FromSIMPLJson()` conversion path, legacy UUID map entry, and a 431-line test file containing one exemplar-based test, four execution tests covering all four template specializations of `FindDensityGrouping<UseNonContiguousNeighbors, FindCheckedFeatures>`, and three preflight error tests. The filter is implemented as the standard Filter+Algorithm pair per the project convention and uses `MessageHelper`/`ThrottledMessenger` for progress reporting and a `m_ShouldCancel` check inside the parent loop.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

line count information here that just complicates the readability. This whole section should just e a checklist the sentences are just bad at clear communication


- **Files in this filter:** filter (.hpp +126, .cpp +278), algorithm (.hpp +52, .cpp +219), doc (.md +90), three doc images (Algorithm.png, FeatureIds.png, ParentIds.png), legacy UUID map (+3 lines), plugin CMakeLists (+2), test CMakeLists (+2), unit test (.cpp +431).
- **Diff size:** 12 files, +1203 lines, –0.
- **Change nature:** **New filter / Port.** Initial introduction of `ComputeGroupingDensityFilter` plus its algorithm, documentation (with worked example and three illustrative images), `FromSIMPLJson()` conversion path, legacy UUID map entry, and a 431-line test file containing one exemplar-based test, four execution tests covering all four template specializations of `FindDensityGrouping<UseNonContiguousNeighbors, FindCheckedFeatures>`, and three preflight error tests. The filter is implemented as the standard Filter+Algorithm pair per the project convention and uses `MessageHelper`/`ThrottledMessenger` for progress reporting and a `m_ShouldCancel` check inside the parent loop.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- **Change nature:** **New filter / Port.** Initial introduction of `ComputeGroupingDensityFilter` plus its algorithm, documentation (with worked example and three illustrative images), `FromSIMPLJson()` conversion path, legacy UUID map entry, and a 431-line test file containing one exemplar-based test, four execution tests covering all four template specializations of `FindDensityGrouping<UseNonContiguousNeighbors, FindCheckedFeatures>`, and three preflight error tests. The filter is implemented as the standard Filter+Algorithm pair per the project convention and uses `MessageHelper`/`ThrottledMessenger` for progress reporting and a `m_ShouldCancel` check inside the parent loop.
#### Change Nature: New Filter / Port
**Mandatory Contains:**
- [x] filter
- [x] algorithm
- [x] test
- [x] documentation
- [ ] pipeline
** Filter Contains:**
- [x] `FromSIMPLJson()` conversion path (if applicable)
- [x] legacy UUID map entry
** Algorithm Contains:**
- [x] cancel checks
- [x] feedback
- [x] progress feedback (if applicable)
** Documentation Contains:**
- [x] worked example
- [x] Images
- [ ] default pipeline
** Test Contains:**
- 3 preflight error tests
- 1 exemplar based test. Test file from version x.x (ex 7.5, 6.6, 6.5, etc)
- 4 execution tests, covering each of the template specializations in the algorithm

Comment on lines +50 to +51
- **Files in this filter:** filter (.hpp +126, .cpp +278), algorithm (.hpp +52, .cpp +219), doc (.md +90), three doc images (Algorithm.png, FeatureIds.png, ParentIds.png), legacy UUID map (+3 lines), plugin CMakeLists (+2), test CMakeLists (+2), unit test (.cpp +431).
- **Diff size:** 12 files, +1203 lines, –0.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

convert the files sentence to a bulleted list of changed files. remove diff size (doesn't really give useful information). Add the commit hash above the affected files section

>
> No broad-refactor PRs (e.g., #1457, #1472, #1501, #1535, #1571, #1582) touched this filter, presumably because they all merged before #1548 or because their sweeps did not reach this newly-added file. There is therefore no "pruned PRs" table at the bottom — there are no pruned PRs.

### PR #1548 — *"FILT: Compute Grouping Density filter added."* — merged 2026-02-25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This whole formatting should be changed from a subheading with a list of lists condensed into sentences to a subsection containing subheadings for each of the bulletpoints converting the sentences into lists in the subsections. see change nature feedback for example

Reviewer noted the first pilot version repeated discovery context
(source files, image filenames, hand-derived constants, test inventory,
algorithm restrictions) across 3-5 phases, making new content hard to
detangle from restated context.

Rewritten so Phase 1 is the single canonical source for discovery
context, and later phases reference Phase 1 by section name rather
than restating. Doc went from 649 to 481 lines (26% shorter) with
no loss of information. Spot-checks: image-filename references went
from 4-5 to 1; restated algorithm summary content collapsed; later
phases now contain only actions and references.

Companion skill update lands in Claude_Support repo (vv-filter
skill): adds "Reference, don't repeat" writing principle and a
canonical-homes table so future runs of vv-filter don't reproduce
the repetition problem on new filters.

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