VV: Read H5OINA Data Fully V&V'ed - #1728
Open
imikejackson wants to merge 10 commits into
Open
Conversation
DREAM3D-NX 7.0.0 through 7.4.1 import Oxford AZtec .h5oina files with four defects in the value-add that sits on top of EbsdLib's H5OINAReader. All four are corrected here and pinned by a Class 1 analytical oracle suite built on hand-authored .h5oina fixtures. Hexagonal alignment. The EDAX/TSL x-axis convention differs from Oxford's by a 30 degree rotation about [0001] applied to phi2. An .h5oina file stores its Euler angles in radians, but the correction added the literal 30 -- thirty radians -- to every Hexagonal_High point, so with the option at its shipped default of ON every hexagonal orientation was wrong. The correction is now 30 degrees expressed in radians, applied on a double intermediate so the stored float32 is the correctly rounded result, which is how the .ctf importer applies the same correction. Multi-scan stacking. When several scans are stacked into one Image Geometry, scan k occupies the tuple slab starting at k * X * Y. The Euler array carries three components per point, but its destination offset was the tuple offset rather than three times it, so from the second scan on the Euler block landed a third of the way into its slab, overwriting the tail of the previous scan and leaving the rest of its own slab zeroed. The hexagonal alignment separately ignored the slab offset and always walked the first scan's points, shifting them once per scan while never reaching the later scans. Both now address the scan's own slab. Pattern import. Pattern data cannot be read from an .h5oina file: the reader's pattern accessors are stubs. Turning on "Import Pattern Data" produced an error claiming the file contained no pattern data, and the execute-side plumbing behind it was internally inconsistent -- preflight created a uint16 array that execute fetched as uint8. The parameter now reports honestly that pattern import is not yet supported for this format, and the unreachable copy block is removed. Malformed-file guards. Six inputs that previously produced an out-of-range read, an out-of-range write or a zero-sized geometry are now rejected with a message naming the offending value, the scan and the file: cell counts below 1 (-9584), a selected scan whose grid differs from the first selected scan's (-9585), a selected scan that is not in the file (-9586), phase groups not numbered 1 through N (-9587), a Data dataset whose extent disagrees with the header's cell counts (-34971), and a phase value outside the range the file's phase definitions establish (-34972). Only the first selected scan used to be checked at all, so a bad later name failed part way through execute with the earlier scans already written. The scan-selection parameter's stacking order is not applied by this filter -- scans are always stacked in list order -- so selecting High To Low now warns (-9588) instead of being silently ignored. The scan loop also reports progress per scan and honours cancellation. The 16 test cases replace an exemplar comparison against a .dream3d file that this filter had itself written. Expected values are derived from the fixture specification; the hexagonal expectations are correctly-rounded IEEE-754 results that distinguish a double intermediate from a float32 one and from the literal 30. The production AZtec file is now checked against a readback of its own datasets rather than against that exemplar. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The documentation described a version restriction the reader does not have, named the wrong Euler angle for the hexagonal alignment, and listed no created outputs at all. - The reader does not gate on the file's Format Version: the value is optional and unused, and the columns read are the Format Version 2.0 set that later versions retain. Replaces the claim that only Format Version 2.0 files are understood. - The hexagonal alignment applies to phi2, the third Euler angle, and only to Hexagonal-High points. Since the file's angles are radians the value added is 30 degrees expressed in radians, not the number 30. - Adds a Created Outputs section naming every array with its type and component count, including that Phase is int32 by default and that tuple 0 of the ensemble arrays is the reserved invalid phase. - Records that the lattice angles are imported as degrees while the Euler angles stay in radians. - Records the multi-scan stacking rules, that the stacking order setting is not applied, and that pattern import is not yet supported for this format. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Report, deviations and provenance sidecar for the H5OINA importer. The filter has no DREAM3D 6.5.171 equivalent -- that tree contains no H5OINA or AZtec importer of any kind -- so no legacy comparison was run and none is possible. Its place is taken by a Class 2 independent readback: the archived production AZtec file is compared against a readback of its own data sets, performed once in the test with H5Lite and once out of band with h5py, rather than against the .dream3d exemplar that ships beside it. That exemplar was written by this filter, so comparing against it pinned the filter to itself, and it carried the pre-correction radian lattice angles, so it actively enforced one of the defects. The deviations file records the seven differences between what DREAM3D-NX 7.0.0 through 7.4.1 shipped and the correct behavior, four in the filter and three in EbsdLib's H5OINAReader, with the affected releases named from docs/dream3d_nx_release_dates.md. Three of them reach users only through EbsdLib 3.1.1, which the report and the deviations both record as a release dependency. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The filter documentation carried the .ctf convention that Error = 0 marks a successfully indexed point. That is not true of this format. In the AZtec export bundled with this filter's tests, all 587 indexed points carry Error = 1 and all 38 un-indexed points carry Error = 2, and no point carries 0, so the masking recipe the page recommended would have selected nothing. The page now recommends thresholding Phase > 0, which is reliable because phase 0 is the reserved invalid-phase slot, and records the observation about Error. The V&V wording is tightened where a claim was looser than the evidence: - The fixture split is recounted from the files the test actually writes: nineteen at run time, eight imported successfully and eleven backing a rejection or passthrough case. The "carries exactly the minimum dataset set" statement is scoped to the eight that import, since two of the others omit a required dataset on purpose. - "Every fixture value is float32-exact" is replaced by what is actually relied on -- every value is a float32 literal stored as float32, so the file round-trips it bit-for-bit. Five of the phi2 values are deliberately not exactly representable decimals, because a dyadic value has trailing zero mantissa bits and cannot separate a double intermediate from a float32 one. - The hexagonal correction is 57.3 times the intended one, not 9.55. - The Euler destination offset was one third of the correct offset; the earlier phrasing implied a position within the destination slab. - The 6.5.171 search result names its single non-source match. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The ensemble fill in the shared IEbsdOemReader::readData runs once per selected scan and writes crystalStructures[phaseId], materialNames[phaseId] and latticeConstants component phaseId, where phaseId is the integer in that scan's HDF5 phase group name. The three ensemble arrays are sized from the FIRST selected scan's phase count alone, so the phase-index guard has to hold for every selected scan and not only for the first. Two shapes of well formed multi-scan file reached that unguarded write: - A later scan carrying a phase group named outside 1..N. This is the same defect the first-scan guard already rejected, simply moved into scan 2. - A later scan declaring more phase groups than the first scan. This needs no malformed input at all: different scans of a real AZtec export may legitimately declare different phase lists. Both now fail preflight. The phase-index check is applied to every selected scan with the bound taken from the first scan's phase count, and a scan whose phase group count differs from the first scan's is rejected with the new code -9589, which also covers the case of a later scan declaring FEWER phases: the shared ensemble arrays would then describe that scan's phases while the earlier scan's points still referred to them. The dataset-extent probe's contract is stated accurately: it re-reads the file's extents because no reader API exposes the buffer sizes the reader allocated, and it rejects a dataset longer than the header describes as well as a shorter one. Its H5ScopedFileSentinel construction no longer casts away a const that the sentinel does not require, and the raw-dataset readback helper in the test no longer writes through a const_cast to a const hid_t, which was undefined behaviour on all nine of its call sites. Fixture B's second scan now differs from the first in its Phase, X and Y columns as well, so a slab-placement error in any of those columns is detectable across the two slabs rather than only in the columns that already differed. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The Stacking Order carried by the scan-selection parameter chooses which end of the selection list lands in tuple slab 0. This filter read it only to decide whether to raise a warning, and stacked the scans in list order either way, so High To Low was a setting the user could choose and the filter would not honour. The scan loop already lives in ReadH5OinaData::operator(), and stackingOrder already reaches the algorithm on ReadH5DataInputValues::SelectedScanNames, whose type OEMEbsdScanSelectionParameter::ValueType carries it. Honouring the setting is therefore a reversal of that loop's iteration order and needs no change to the shared IEbsdOemReader header, so the two sibling OEM readers are unaffected. Low To High reads the scans in the order they are listed; High To Low reads them in the reverse of that order, so the last selected scan occupies slab 0. Warning -9588, which told the user to reverse the scan selection by hand, is retired. Because the destination slab index no longer matches a scan's position in the selection list, the algorithm carries the name of the scan currently in the reader's buffers rather than re-deriving it by walking the selection list, which also makes the per-scan lookup constant time instead of linear. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The lattice angles of an H5OINA import are reported in degrees, matching every other EBSD importer. DREAM3D-NX 7.0.0 through 7.4.1 reported them in radians, so the three angle slots of the LatticeConstants array change value for every H5OINA import: any .dream3d file, regression baseline or pipeline comparison produced by those releases reads a different number after the change. The page now says so where it describes the unit, and carries a Migration Notes section covering both that change and the hexagonal alignment correction, in the form the sibling ReadCtfData page uses. The Format Version paragraph is scoped to what is actually evidenced: the reader reads a fixed key and column set, which is the set Oxford documents for FORMAT VERSION 2.0 and which the bundled Format Version 5.0 export still carries. The earlier wording asserted that all later versions retain it. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The deliverables now record eleven deviations rather than seven. Four were missing: - D8, the multi-scan phase-group write. A selection whose scans do not all declare the same phase groups crashed the process, from a well formed file. The report previously claimed this filter's preflight prevented the unguarded ensemble write, which was true only for the first selected scan. - D9, the Stacking Order setting, which was accepted and never applied. - D10 and D11, the two H5OINAReader corrections that carried user-visible behavior changes -- every composed error message discarded, and readData() returning a code that did not match the one it set while never validating its column count -- neither of which had an entry. D6 is labelled a breaking change and carries a release note and migration paragraph: it changes the value of a published output array for every H5OINA import, and no released version carries it. Two coverage justifications were false. Row 9's later-scan -9582 return is a different statement in a different object from row 6's, not the same one, and row 4's note named the wrong rows. Row 9 is now covered by a test section that asserts the scan name that site injects, and row 4 is enumerated as the genuinely untested site it is. The table gains the second -34971 return, the extent probe's skip branch, the stacking-order path, the -9589 rejection and the three EbsdLib phase-read codes with no constructible fixture: 23 of 30, not 21 of 26. Every count is recounted against the sources: 23 fixture files of which eight import and fifteen back a rejection or passthrough; 17 test cases; five discriminating phi2 values and three dyadic controls, not eight discriminators; eight arrays beside Euler in the multi-scan comparison, not five; the assertion count given both as measured in one process and as summed over ctest's per-process runs, with the reason they differ. The claim that the OrientationAnalysis suite carries 29 pre-existing failures does not survive a full build of the preset directory. Twenty-eight of them were PIPELINE:: and PY:: tests, which run nxrunner and the Python bindings -- targets that building only the unit-test target leaves stale. After a full build the suite is 308 tests with one failure, ComputeSchmidsFilter, which is the known EbsdLib CubicOps precision drift, and SimplnxCore is 985 of 985. The provenance records that, and keeps the controlled EbsdLib rollback for what it does establish: rolling EbsdLib back to the batch base changes the non-H5OINA failure set not at all. The provenance sidecar states once that its evidence filenames live in an uncommitted working folder archived to the V&V remote, and no committed document cites a path under that folder or under .superpowers/. It also gains the Generated by / Generated on / canonical-oracle rows the provenance template requires, the EbsdLib and h5py/NumPy versions behind the trusted boundary, and a complete list of the suite logs. Wording throughout states what the code does rather than which pass changed it, per BlueQuartzSoftware#1725; the exemplar bit-identity claim is scoped to the Euler array, which is what was measured. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The H5OINA importer's correct output depends on three H5OINAReader corrections that ship in EbsdLib 3.1.1: the third lattice angle is stored in the gamma slot instead of gamma echoing beta, the lattice angles are converted from the file's radians to the degrees that every other importer reports, and the phase dataset reads are checked so that a phase group missing its Lattice Angles is reported rather than crashing the process. Built against EbsdLib 3.1.0 the filter reports the wrong gamma, reports the angles in the wrong unit, and still crashes on that malformed file, and three of the filter's tests fail. Raising the pin makes that a configure-time failure rather than a silent behavioural difference. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
…ction * Every reproduction of the D8 defect crashed before writing output, but the write is undefined behavior, so the recommendation no longer claims the crash is the only possible outcome * Disclose the residual restriction: the guards compare phase-group names and counts across scans, not their contents; differing definitions keep the last scan's values and should be imported separately * Repair the sibling-exposure follow-up's sentence structure Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
imikejackson
force-pushed
the
vv/ReadH5OinaData
branch
from
August 25, 2026 21:49
7191ad5 to
b73fd43
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
1 participant
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.
Summary
Full oracle-first V&V of
ReadH5OinaDataFilterper the MTR V&V policy (docs/vv_templates/). This is the program's first new-filter V&V — no DREAM3D 6.5.171 equivalent exists — so the legacy binary A/B is replaced by a Class 2 independent readback: an h5py script re-reads every fixture and the real AZtec file and independently derives the expected arrays. Report, deviations, and provenance atsrc/Plugins/OrientationAnalysis/vv/.vcpkg.jsonto EbsdLib>= 3.1.1. Five of the corrections live in EbsdLib'sH5OinaReaderontopic/3_1_1_stagingand reach users only through that release (which also gates #1723). Until 3.1.1 is published, this branch's HEAD does not configure; all test evidence is recorded at HEAD−1 against the local staging EbsdLib.Eleven defects corrected (6 filter, 5 EbsdLib), each pinned by a regression test:
30.0— thirty radians — to radian-valued φ2, producing garbage orientations for every hexagonal point in every release that shipped this filter. The alignment now adds the intended 30° (π/6). Invisible until now because the only test file was cubic.-9587,-9589).H5OinaReader(D5, D6, D7, D10, D11): the lattice-constant γ slot echoed β; lattice angles were imported in radians where every other importer reports degrees —Lattice Anglescrashed the process; error-path hygiene.Tests: 2 → 17 test cases (9,374 assertions): the circular NX-generated exemplar comparison was replaced by readback-derived assertions against the genuine AZtec file; hand-built h5py fixtures cover hex alignment (pinned to numerically distinguish the correct π/6 from the former literal), multi-scan stacking, guard cases, and the real-file import. Mutation-verified. Also fixes the invalid-input TEST_CASE that referenced a file present in no archive (it passed vacuously).
Docs: the format-version claim, the CTF copy-paste alignment paragraph, the created-objects table, and a masking recipe that selected nothing on real data are corrected.
Follow-ups (in the report):
ReadH5OimData/ReadH5EspritDatashare several of these defect shapes viaIEbsdOemReaderand are enumerated for the engineer track; closing the preflight-vs-execute gap at the shared write needs a cross-filter change.Test Plan
ctest -R "OrientationAnalysis::ReadH5Oina"— 17/17 (HEAD−1, local EbsdLib 3.1.1 staging)OrientationAnalysis::— 308 tests, 1 pre-existing failure (ComputeSchmidsFilter, the known CubicOps drift corrected by VV: Compute Schmids Fully V&V'ed #1723);SimplnxCore::— 985/985