Skip to content

REV: ComputeAvgOrientations V&V follow-ups (guards, tolerances, provenance) #1661

Description

@imikejackson

Minor follow-ups from the adversarial post-merge review of #1645 (ComputeAvgOrientations, merged to develop). Grouped here to avoid issue spam; split out if any warrants its own PR.

Filter: src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ComputeAvgOrientations.cpp, test .../test/ComputeAvgOrientationsTest.cpp, deliverables under .../vv/.

  • Untested new guards + silent drop. The two out-of-range crystal-structure guards added by the PR (ComputeAvgOrientations.cpp:85-88 and :435-438) are not exercised (both fixtures put 999 only at ensemble index 0, never dereferenced), and they silently drop voxels/features with an unknown crystal structure — no warning is emitted and the docs don't say so. Add coverage + a warning, or document the drop.
  • Phase index unguarded (OOB read). xtalRef[phaseValue] (:82) and crystalStructures[currentPhase] (:433) are not range-checked; a cell Phases value ≥ the ensemble tuple count is an out-of-range read. Preflight's -651 validates tuple counts, not phase values.
  • Tolerance provenance mismatch. vv/provenance/ComputeAvgOrientationsFilter.md states quaternion ±1e-6 / kappa ±1e-4 relative, but the test uses k_QuatMargin = 5.0e-3f and k_KappaRel = 0.02. Reconcile the doc to the shipped tolerances (or tighten the test) and justify the 2% kappa margin.
  • Documented assertions that don't exist. The provenance claims a "Euler invariant (Φ≈0, all finite)" check for z-rotation fixtures and a "voxel-ordering independence (verified)" reordered-fixture test; neither is encoded in the test. Either add them or remove the claims.
  • featureIdToPhaseMap last-writer-wins for a multi-phase feature (vMF/Watson uses one phase per feature; Rodrigues is per-voxel) — undocumented; note it.
  • Report self-contradictions: "4/4" vs 5 tests; Oracle section "OOC dual-build confirmed in Phase 8" vs V&V-phase "OOC build skipped"; a never-written vv/provenance/7_ComputeAvgOrientation_v2.md reference. Clean up.
  • Error tests don't assert codes (-54670 no-method-enabled, -651); and a duplicate -651 test now coexists after TEST: Add coverage tests for filters flagged by the GCOV report #1644. Assert codes and de-duplicate.
  • QUESTION: preflight passes with all three averaging methods disabled, deferring to a guaranteed runtime -54670. Consider making it a preflight error so the GUI surfaces it before execution.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions