Skip to content

Commit ce3d290

Browse files
imikejacksonclaude
andcommitted
DOC: PR #1631 review feedback — V&V report wordiness tightening
Addresses @nyoungbq's PR-review observation that the V&V deliverables "feel overly wordy." Targeted scrub across all 7 V&V'ed filters in this PR. No load-bearing evidence removed: - "At a glance" cells reduced from 1000-2200 characters down to 300-800 characters. The table is meant to be scannable; oversized cells defeat that purpose. Detail relocated to the long-form sections that already carried it. - Trim restatement between Summary, Algorithm Relationship Evidence, and the Port-time-deltas list. Each fact now appears in exactly one canonical place; later sections reference rather than repeat. - Compress the Pure-Φ rotation closed-form argument in ComputeFeatureNeighborCAxisMisalignmentsFilter.md from a 17-line derivation block (restated 3 times across the file) to a single 4-line argument referenced by the per-fixture table. Total content preserved: deviation tables, oracle derivations, commit hashes, SHA512s, code-path coverage tables, test inventories, empirical A/B byte-match tables. All 7 reports retain SBIR-deliverable rigor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent afd06e2 commit ce3d290

7 files changed

Lines changed: 28 additions & 55 deletions

src/Plugins/OrientationAnalysis/vv/BadDataNeighborOrientationCheckFilter.md

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
| Aspect | Current state |
1616
|------------------------|--|
17-
| Algorithm Relationship | **Port** of legacy `BadDataNeighborOrientationCheck::execute()` (DREAM3D 6.5.171). Same two-pass structure: (1) per-voxel face-neighbor count of within-tolerance misorientations; (2) iterative-decay pass that flips a bad voxel when its same-phase good-neighbor count meets the current level. SIMPLNX bundles two substantive bug fixes from PR #1499 (Issue 1: convergence-loop bound `>``>=`; Issue 2: misorientation-threshold check moved inside same-phase conditional) plus a Phase-6 SIMPLNX tolerance-precision fix. |
17+
| Algorithm Relationship | **Port** of legacy `BadDataNeighborOrientationCheck::execute()`. Same two-pass iterative-decay structure; SIMPLNX bundles two legacy bug fixes (D1, D2 — PR #1499) and a SIMPLNX-side float-π precision fix (Phase 6 of this V&V cycle). |
1818
| Oracle (confirmed) | **Class 1 (Analytical) primary** — engineer's hand-derived `expectedMask` arrays for all 27 algorithmic fixtures, mirrored from `bad_data_neighbor_orientation_check_v2/test_design.md`. **Class 4 (Invariant) companion** — monotonicity + no-degrade asserted via `ClassFourInvariants` helper across all base fixtures and a dedicated idempotence test. |
1919
| Code paths enumerated | 7 of 7 algorithmic paths exercised (cancel check, mask-skip, mixed-phase skip, background-voxel skip, within-tolerance increment, above-tolerance skip, iterative-decay flip + neighbor-count update). |
2020
| Tests today | **31 TEST_CASEs / 49 ctest entries**, 100% pass (2.40s). 27 Class 1 base + 1 SIMPL backwards-compat + 1 Class 4 Invariants Sweep (18 DYNAMIC_SECTIONs) + 1 Class 4 Idempotence + 1 2D Image Fixture (inline-constructed). |
@@ -25,17 +25,13 @@
2525

2626
## Summary
2727

28-
`BadDataNeighborOrientationCheckFilter` iteratively flips "bad" voxels in a `Mask` array to "good" if a sufficient number of their same-phase face neighbors have crystallographically-similar orientations. The algorithm runs in two passes: (1) for each masked-false voxel, count how many of its 6 face neighbors fall within the user-supplied misorientation tolerance (computed per the appropriate Laue-group symmetry); (2) iterate `currentLevel = 6` down to the user-supplied `NumberOfNeighbors`, flipping every voxel whose count meets the current level and updating its still-bad neighbors' counts after each flip — producing a flood-fill behavior that converges when no flips occur at the user-supplied lower bound.
29-
30-
Verification is via a **Class 1 (Analytical) hand-derived dataset of 27 algorithmic fixtures** (3×3×3 base cases for parameter combinations of `MisorientationTolerance`, `NumberOfNeighbors`, and phase configuration, plus 5×5×5 sequential / recursive / semi-complex fixtures), with expected `Mask` outputs encoded inline as `std::array<uint8, N> expectedMask` literals in the test source. A **Class 4 (Invariant) companion oracle** adds monotonicity + no-degrade + idempotence assertions across all base fixtures. A **direct A/B comparison against DREAM3D 6.5.171** on the same 27 fixtures runs through the official `PipelineRunner` and confirms two legacy defects (D1: convergence-loop bound off-by-one; D2: stale `w` variable across mixed-phase neighbors) that the SIMPLNX rewrite correctly fixes; both are documented as Deviations.
31-
32-
A SIMPLNX-side precision bug was uncovered and fixed during this V&V cycle: the misorientation tolerance was computed as `MisorientationTolerance × numbers::pi_v<float> / 180.0f`, but `pi_v<float>` is slightly larger than true π, making the float-radian tolerance ~5e-9 rad larger than mathematically faithful. For 4 boundary-exact Case 1.X.3 fixtures landing on *exactly* the user-supplied tolerance, the float-π conversion incorrectly included misorientations that should fail strict `<`. The fix promotes the tolerance computation to `double` + `numbers::pi_v<double>`. With this fix in place, SIMPLNX is bit-identical to the engineer's hand-derived oracle across all 31 tests.
28+
`BadDataNeighborOrientationCheckFilter` iteratively flips "bad" voxels in a `Mask` array to "good" if a sufficient number of their same-phase face neighbors have crystallographically-similar orientations within the user-supplied misorientation tolerance. Verification used a **Class 1 (Analytical) hand-derived dataset of 27 algorithmic fixtures** with expected `Mask` outputs inline in the test source as `std::array<uint8, N>` literals, plus a **Class 4 (Invariant) companion oracle** asserting monotonicity, no-degrade, and idempotence. A direct A/B against DREAM3D 6.5.171 confirms the SIMPLNX rewrite correctly fixes two legacy defects (D1, D2 — documented as Deviations) and a SIMPLNX-side float-π precision bug surfaced during this V&V cycle (tolerance computation promoted from `float` + `numbers::pi_v<float>` to `double` + `numbers::pi_v<double>` to remove a ~5e-9 rad amplification at the exact-tolerance boundary). All 31 tests pass bit-identical to the analytical oracle.
3329

3430
## Algorithm Relationship
3531

3632
*Classification:* **Port** ~~| Minor changes | Rewrite | New filter~~
3733

38-
*Evidence:* The SIMPLNX algorithm at `src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/BadDataNeighborOrientationCheck.cpp` (~260 lines) is a near line-by-line translation of legacy `BadDataNeighborOrientationCheck::execute()` (DREAM3D 6.5.171, ~240 algorithm lines). Same SIMPL UUID retained via `OrientationAnalysisLegacyUUIDMapping.hpp` + SIMPL 6.4/6.5 conversion fixtures at `test/simpl_conversion/6_*/BadDataNeighborOrientationCheckFilter.json`. Same two-pass control flow: (a) initial per-voxel face-neighbor scan that populates a `neighborCount[]` array; (b) outer while-loop iterating `currentLevel = 6 → NumberOfNeighbors`, with an inner while-loop running until no more flips occur at the current level. Per-voxel logic in both passes is mathematically identical: extract the voxel's quaternion, compute misorientation to each of 6 face neighbors via `LaueOps::calculateMisorientation`, count those within `MisorientationTolerance`.
34+
*Evidence:* Near line-by-line translation of legacy `BadDataNeighborOrientationCheck::execute()`. Same SIMPL UUID retained; SIMPL 6.4/6.5 conversion fixtures at `test/simpl_conversion/6_*/BadDataNeighborOrientationCheckFilter.json`. Two-pass control flow and per-voxel misorientation logic are preserved (see Summary).
3935

4036
*Port-time deltas (each tracked as a Deviation or Non-deviation — see `vv/deviations/BadDataNeighborOrientationCheckFilter.md`):*
4137

src/Plugins/OrientationAnalysis/vv/ComputeAvgCAxesFilter.md

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414

1515
| Aspect | Current state |
1616
|------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
17-
| Algorithm Relationship | **Port** with 3 PR-#1438 intentional changes (float→double accumulation, `counter[]++` reorder, error-code re-numbering / preflight-warning removed) + PR #1472 EbsdLib 2.0 refactor + PR #1582 cancel-check additions + Phase-7 V&V refactor (`counter==0`NaN at finalize + final per-feature normalize) |
18-
| Oracle (confirmed) | **Class 1 (Analytical) primary** — 11-cell hand-built dataset with closed-form expected `AvgCAxes` per feature, covering 8 of 12 code paths (gaps: all-hex-ensemble preflight, background-voxel skip, and the two cancel-check paths). **Class 4 (Invariant) companion** — `||AvgCAxes[i]|| == 1.0` for hex-valid features (post-normalize unit-vector contract), `NaN` whenever `counter[feature] == 0` at finalize (placeholder feature, no-cells feature, all-non-hex feature). F7's direction is implementation-dependent at the precision-sensitive antipodal-flip boundary; only the unit-vector magnitude is asserted. (Class 3 dropped — standard/Bunge inform the quaternion-to-matrix convention but no paper has a worked example for this specific filter; hand-derivation is more direct.) |
17+
| Algorithm Relationship | **Port with Minor Changes**float→double accumulation + counter-reorder + error-code re-numbering (PR #1438); EbsdLib 2.0 API (PR #1472); cancel checks (PR #1582); `counter==0`NaN finalize + per-feature normalize (V&V refactor). 7 deltas total — see Algorithm Relationship. |
18+
| Oracle (confirmed) | **Class 1 (Analytical)** — 11-cell hand-built dataset, closed-form `AvgCAxes` per feature, 8 of 12 code paths covered. **Class 4 (Invariant)** — `||AvgCAxes||==1.0` for hex-valid; `NaN` for empty/non-hex. F7 only asserts unit-vector magnitude (direction is precision-sensitive at the antipodal-flip cancellation boundary). Class 3 N/A. |
1919
| Code paths enumerated | 12 (from line-by-line scan of `ComputeAvgCAxes.cpp`) |
2020
| Tests today | 3: 1 valid-execution exemplar (positive), 1 all-non-hex error (negative), 1 SIMPL 6.4+6.5 backwards-compat (DYNAMIC_SECTION) |
2121
| Exemplar archive | **`7_2_AvgCAxis.tar.gz` retired** — confirmed legacy-by-reputation oracle: reference values produced by a "special build of DREAM3D 6.6.379 with micro-texture bug fixes," not by an independent oracle. Per policy line 33, not eligible as a correctness oracle. Replaced by `compute_avg_c_axis.tar.gz` (hand-built Class 1 dataset, this V&V cycle). |
@@ -60,19 +60,13 @@
6060
- *(excluded — doc typo)* #1547 — "DOC: Fix filter documentation and documentation related code bugs" (2026-03-10) — docs `+1/-1` ("Crystallographic" → "Crystallography" subgroup typo). No algorithm or API change.
6161
- *(pruned — broad refactor, no behavioral change to this filter)* #1439 (NeighborList tuple API), #1457 (static-inline cleanup), #1501 (Vec3 unification), #1538 (zlib tar.gz extraction in tests).
6262

63-
<!-- end vv-discover DRAFT -->
64-
6563
## Oracle
6664

6765
*Class:* **1 (Analytical)** primary, **4 (Invariant)** companion.
6866

69-
### Phase 2 exemplar-provenance finding (recorded for the audit trail)
70-
71-
The pre-existing primary exemplar `7_2_AvgCAxis.tar.gz` was investigated per the "circular oracle" cross-cutting finding from the retroactive audit's INDEX. Its inline `ReadMe.md` confirms:
72-
73-
> Processed with [a] special build of DREAM3D Version 6.6.379 (or close to that) — this version had specific bug fixes for micro-texture data. Used the output file from the 6.6.379 version as the input into DREAM3D-NX version 7.3.0. Final output file has results from both versions. All results should be within 5.0E-7 tolerance of each other.
67+
### Phase 2 exemplar-provenance finding
7468

75-
This makes `7_2_AvgCAxis.tar.gz` a **regression-style oracle pinned to a 6.6.379 special build** — closer to Class 5 (regression / golden-master with a "trust the legacy" assumption) than to any of the policy's preferred Class 1–4 oracles. The DREAM3D 6.6.379 special build itself was never independently verified, and per V&V policy line 33 ("Legacy 6.5.171 produced this output is never a valid oracle for correctness") this is doubly disqualified: it's not even 6.5.171 baseline — it's a later, divergent special build. The exemplar is **retired** as the primary oracle for this filter.
69+
The pre-existing exemplar `7_2_AvgCAxis.tar.gz` is a regression-style oracle pinned to a custom DREAM3D 6.6.379 "micro-texture special build" (per its inline ReadMe). Per V&V policy line 33, legacy output is never a valid correctness oracle — doubly disqualified here, since the reference build isn't even 6.5.171 baseline. **Retired** in favor of the hand-built Class 1 dataset below.
7670

7771
### Applied (Class 1 — Analytical)
7872

0 commit comments

Comments
 (0)