BUG: H5OINA reader fixes for the 3.1.2 release - #56
Merged
imikejackson merged 8 commits intoAug 27, 2026
Merged
Conversation
H5OINAReader::readHeader() assembled each phase's lattice constants as
{a, b, c, alpha, beta, beta} -- the second lattice angle was written into
both the beta and the gamma slot and the third angle read from the file
was never used. Any phase whose gamma differs from its beta, such as a
hexagonal phase at 90/90/120, therefore reported the wrong gamma to
every consumer of CtfPhase::getLatticeConstants().
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
An H5OINA file stores its phase lattice angles in radians. H5OINAReader copied them into CtfPhase's lattice constants unchanged, while every other EbsdLib importer -- .ang, .ctf and their HDF5 variants -- fills those same three slots with degrees. A consumer reading LatticeConstants therefore got a different unit depending on which file format the phase came from, with a cubic phase reporting 1.5707964 instead of 90. The angles are now converted on import, so the phase model is format-independent. The conversion uses a double intermediate so the stored float is the correctly rounded result. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The Lattice Dimensions, Lattice Angles, Laue Group and Space Group reads discarded their error codes, and the phase group open was never checked. A file whose phase group was missing one of the two vector datasets left the corresponding std::vector empty and the next statement indexed elements 0 through 2 of it, which crashed rather than reporting the malformed file. The four required reads are now checked and reported with their own error codes (-90030 through -90033) naming the phase and the dataset. Space Group stays optional, since the reader only passes it through. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Ten of the reader's failure paths declared an empty std::string, built a std::stringstream *from a copy of it*, streamed the message into the stream and then handed the still-empty original to setErrorMessage(). The composed text was discarded, so every one of those failures surfaced with an empty message -- including the header-read failure that callers report to the user. The message is now taken from the stream. Two related fixes ride along because an empty message hid them: readFile()'s generic "could not read header" / "could not read data" wrappers replaced the specific message that readHeader() or readData() had already set, so they now name the scan and append that message; and the class name ran into the word "Error" without a space. An unused stream buffer in readData() is removed. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
The zero-size rejection set error code -90301 but returned -301, so a caller that reported the returned value and a caller that read getErrorCode() disagreed about which failure had occurred. Both now report -90301. The column count was also never validated on its own. X Cells and Y Cells are signed values from the header; a negative column count was widened straight to size_t, producing an enormous column count whose product with the row count could be non-zero and slip past the zero-size check. Both counts are now checked while still signed and the messages name the offending header key and its value. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
H5OINAReader.cpp has no namespace block, so the radians-to-degrees helper is a generically named function with external linkage in a shared library. Any other translation unit in EbsdLib or in a consuming project that defines RadiansToDegrees(float) at global scope is then a duplicate symbol at link time or an ODR violation. The helper has one call site in this file, so it is marked static. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
* Bump the project version from 3.1.1 to 3.1.2 * Add the 3.1.2 release notes covering the H5OINA reader fixes Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
* Parse phase group names without throwing exceptions * Reject noncanonical names before opening the phase group Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
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
The 3.1.1 release (#54) picked up the MDF/Texture and Laue-class work from
topic/3_1_2_staging, but the H5OINA reader commits on that branch were left out. This PR carries those six commits forward ontodevelopand bumps the version to 3.1.2.Source/EbsdLib/IO/HKL/H5OINAReader.cppis the only source file changed — a content diff againstrelease/v3.1.1confirms it is the sole substantive gap between that tag and the staging branch (the remaining differences were clang-format whitespace noise).Lattice parameters
readHeader()stored the second lattice angle in both the beta and gamma slots, so the gamma angle of every H5OINA phase was wrong for any non-cubic cell. The third angle now lands in the gamma slot.CtfPhase's lattice constants are degrees-valued for every other importer (.ang,.ctf, and their HDF5 variants). The reader now converts on import so the phase model is consistent regardless of source format. The conversion runs through adoubleintermediate so the storedfloatis correctly rounded.Unvalidated reads
LatticeDimensions,LatticeAngles, andLaueGroupphase datasets were read without checking the returned error code. A missing or short dataset left the vectors empty and the indexing that followed was undefined behavior rather than a reported error. Each read is now checked for both a successful status and a sufficient element count, and returns a distinct error code.SpaceGroupstays optional.H5Gopen()on a phase group is now detected instead of being handed to later calls as a negative handle.readData()widened the signedXCells/YCellsheader counts tosize_tbefore validating them, so a negative count became an enormous unsigned value and the row-times-column product was meaningless. Both counts are validated while still signed, and the column count is now checked independently of the row count.Error reporting
setErrorMessage()call in the reader built astd::stringstreamfrom an emptystd::string, filled the stream, then passed the still-empty original string tosetErrorMessage(). Every error this reader produced was blank. The composed message is now what gets reported.Release
project(EbsdLibProj VERSION 3.1.2)Docs/Index.mdTest Plan
ci-macos-arm64preset (159/159 targets, no errors)ctest— 408/408 tests passclang-format19 (the version pinned by the format workflows) reports the changed file as clean