VV: Filter V&V Action List#1611
Conversation
274f2f8 to
62b68c6
Compare
|
|
||
| ## Thread 1 — Oracle versioning (preventing "oracle drift") | ||
|
|
||
| Each oracle class has a different fragility profile: |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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>
ae2e8d0 to
8071cad
Compare
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. |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
| - **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 |
| - **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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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>
No description provided.