VV: ComputeFaceIPFColor is completely V&V'ed#1646
Open
imikejackson wants to merge 3 commits into
Open
Conversation
bf94217 to
31adfca
Compare
This was referenced Jul 1, 2026
nyoungbq
reviewed
Jul 3, 2026
nyoungbq
left a comment
Contributor
There was a problem hiding this comment.
All looks good. Was the vv_work directory meant to be left in, if yes I will approve it to go in Tuesday morning otherwise remove. If yes I can also dump what I used for the v&v A/B Fill Bad Data in a directory in my PR too
31adfca to
eb19efe
Compare
…ce side Fixes the issue BlueQuartzSoftware#1635 wrong-phase mis-coloring. The Phase-2 branch of CalculateFaceIPFColorsImpl indexed m_CrystalStructures[phase1] in both the crystal-symmetry validity guard and the IPF operator lookup, while correctly reading feature2's Euler angles and the flipped face normal. It therefore applied Phase 1's symmetry operators to Phase 2's orientation. * Change the two phase1 indices to phase2 (Algorithms/ComputeFaceIPFColoring.cpp) and remove the KNOWN BUG block. Matches the legacy 6.5.172 backport 1c96b3b8e. * Two manifestations are corrected: wrong Laue operator on mixed-Laue-group faces, and black second color on faces whose first side is the exterior (feature1 <= 0) — the latter affects even single-phase data. * Replace the circular "Valid filter execution" test (it compared against the filter's own pre-fix output baked into 6_6_Small_IN100_GBCD) with a Class 1 analytical test: a hand-built 4-face cubic/hex mesh whose expected colors are closed-form IPF standard-triangle corner primaries. Verified to fail when the bug is reintroduced. V&V (issue BlueQuartzSoftware#1635): * Three-way legacy binary A/B (6.5.171 / 6.5.172 / nxrunner) on a legacy-native cubic/hex fixture: 6.5.171 reproduces the buggy oracle; 6.5.172 and SIMPLNX both apply the hex operator (D1 fixed). Real 756,474-face A/B: SIMPLNX-fixed vs the baked buggy baseline differs on exactly 120,000 feature1<=0 boundary faces, 0 elsewhere. * Deviation D2 (library): EbsdLib colors the hex basal direction green; legacy DREAM3D's older EbsdLib colors it blue (cubic unaffected). Resolved: EbsdLib is canonical and 3.0.0 and 3.1.0 agree on green (the Class 1 oracle passes unchanged on a from-source EbsdLib 3.1.0 build); legacy blue is the deviation, trust SIMPLNX. The convention-independent oracle assertion is red-channel==0. * Add V&V deliverables: report, deviations (D1 + D2), provenance note, legacy comparison summary, and reproducible fixture/pipelines. * Clarify per-phase symmetry behavior and the 6.5.171 migration note in docs. Note: the NX-written 6_6_Small_IN100_GBCD surface mesh is not readable by legacy DataContainerReader (DataArray<size_t> connectivity); the legacy fixture was authored with the legacy_dream3d writer. Worth a separate NX-export issue. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Reword the legacy-comparison notes for ComputeFaceIPFColoring: SIMPLNX-written .dream3d files are intentionally not readable by legacy DREAM3D 6.5.171 (the FileVersion dataset gates it). Feeding an NX file to the legacy PipelineRunner is not a supported path, so authoring the legacy A/B input natively with the legacy_dream3d writer is the standard approach -- not a workaround for a bug. Removes the incorrect "NX->legacy read gap / worth filing as its own issue" framing. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
eb19efe to
9c1cdc3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dependencies
Summary
Full V&V of
ComputeFaceIPFColoringFilter, which fixes the latent mixed-phase mis-coloring bug tracked in #1635 and lands the verification deliverables (report, deviations, provenance). Also bumps EbsdLib to 3.1.0, which is now treated as the canonical IPF-color authority.Fixes #1635
The fix (deviation D1)
The Phase-2 branch of
CalculateFaceIPFColoring::generateindexedm_CrystalStructures[phase1]in both the crystal-symmetry validity guard and the IPF-operator lookup, while correctly readingfeature2's Euler angles and the flipped face normal. It therefore applied Phase 1's symmetry operators to Phase 2's orientation. Changed the two indices tophase2(matching the legacy 6.5.172 backport1c96b3b8e) and removed the// KNOWN BUGblock.Two manifestations are corrected:
feature1-invalid boundary faces (label ≤ 0): the buggy guard read the unknown-phase sentinel and failed, leaving the second color black. This affects even single-phase data.Verification (Class 1 analytical oracle)
Class 1 Oracle - mixed-phase analytical: a hand-built 4-face cubic/hex mesh whose expected colors are the closed-form IPF standard-triangle corner primaries (cubic<100>→red,<111>→blue; hex c-axis→red, basal→green with red-channel exactly 0). Independent of any DREAM3D/EbsdLib implementation. Verified to fail when thephase1→phase2fix is reverted.Valid filter executiontest: it compared against aSurfaceMeshFaceIPFColorsarray baked into6_6_Small_IN100_GBCD— the filter's own pre-fix output (a circular oracle that encoded the bug). Real-data exercise is retained by the Color-Key plumbing test, which runs on the full Small IN100 GBCD mesh.Legacy A/B (three-way: 6.5.171 / 6.5.172 / SIMPLNX)
Run on a legacy-native cubic/hex fixture authored with the
legacy_dream3dPython writer, plus SIMPLNX-vs-baked on the real 756,474-face mesh:feature1 ≤ 0boundary faces and nowhere else.(0,255,0); legacy DREAM3D's older EbsdLib colors it blue(0,0,255). Cubic coloring is identical across all three. EbsdLib is canonical and 3.0.0 and 3.1.0 agree on green (the Class 1 oracle passes unchanged on a from-source 3.1.0 build), so legacy's blue is the deviation — trust SIMPLNX. Migration note: hexagonal IPF hues change vs. legacy DREAM3D; this is expected and correct.EbsdLib 3.1.0
Bumps the EbsdLib dependency to 3.1.0 (canonical IPF-color reference). The
ComputeFaceIPFColoringtests pass identically under EbsdLib 3.0.0 and 3.1.0.Deliverables
src/Plugins/OrientationAnalysis/vv/ComputeFaceIPFColoringFilter.md— V&V report (Status: READY FOR REVIEW)src/Plugins/OrientationAnalysis/vv/deviations/ComputeFaceIPFColoringFilter.md— D1 (bug, fixed) + D2 (library, resolved)src/Plugins/OrientationAnalysis/vv/provenance/…-circular-oracle.md— documents the retired circular exemplarvv_work/face_ipf/— reproducible legacy/NX pipelines, fixture builder, and the legacy-comparison summaryTest Plan
ComputeFaceIPFColoringtests pass (in-core build, EbsdLib 3.0.0)ComputeFaceIPFColoringtests pass on a from-source EbsdLib 3.1.0 buildtopic/write_pole_figure_fixesis indevelop(required for the 3.1.0 build)Follow-ups (separate issues)
DataArray<size_t>, which legacyDataContainerReadercannot parse, so NX-written surface meshes don't read back in legacy DREAM3D.