From e2b4e1cbca9456348ed78baa4f61c4f8f34587d9 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Tue, 7 Jul 2026 09:13:32 -0400 Subject: [PATCH 1/3] VV: Read EDAX EBSD Data (.ang) fully V&V'ed Summary: - Found and fixed 1 bug (out-of-bounds ensemble write on non-contiguous phase indices; preflight now sizes by max phase index, all slots get Invalid-Phase defaults, execute guards the range with -19502); - documented 4 deviations from DREAM3D 6.5.171 (D1 MaterialName trailing space, D2 TEM/ACOM Nanometer units dropped, D3 legacy segfault on non-contiguous phase indices, D4 error-code renumbering); - retired 3 tests (circular-oracle exemplar consumer + 2 archive-based error tests) and the read_ang_test.tar.gz archive itself; - unit tests replaced with 7 inlined *Class 1 (Analytical) + Class 4 (Invariant)* test fixtures (incl. new SIMPL backwards-compat test); - added 3 V&V source-tree deliverables (report, deviations, provenance); - fixed doc typos, added created-arrays table, units note, and current example-pipeline list; removed dead legacy PIMPL cache declarations. Signed-off-by: Michael Jackson --- .../docs/ReadAngDataFilter.md | 38 +- .../Filters/Algorithms/ReadAngData.cpp | 88 ++-- .../Filters/Algorithms/ReadAngData.hpp | 47 +- .../Filters/ReadAngDataFilter.cpp | 17 +- .../Filters/ReadAngDataFilter.hpp | 8 - .../OrientationAnalysis/test/CMakeLists.txt | 1 - .../test/ReadAngDataTest.cpp | 411 ++++++++++++++++-- .../vv/ReadAngDataFilter.md | 130 ++++++ .../vv/deviations/ReadAngDataFilter.md | 80 ++++ .../vv/provenance/read_ang_test.md | 40 ++ 10 files changed, 718 insertions(+), 142 deletions(-) create mode 100644 src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md create mode 100644 src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md create mode 100644 src/Plugins/OrientationAnalysis/vv/provenance/read_ang_test.md diff --git a/src/Plugins/OrientationAnalysis/docs/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/docs/ReadAngDataFilter.md index 0954623650..4639bd6ea0 100644 --- a/src/Plugins/OrientationAnalysis/docs/ReadAngDataFilter.md +++ b/src/Plugins/OrientationAnalysis/docs/ReadAngDataFilter.md @@ -6,11 +6,28 @@ IO (Input) ## Description -This **Filter** will read a single .ang file into a new **Image Geometry**, allowing the immediate use of **Filters** on the data instead of having to generate the intermediate .h5ebsd file. A **Cell Attribute Matrix** and Ensemble Attribute Matrix** will also be created to hold the imported EBSD information. Currently, the user has no control over the names of the created **Attribute Arrays**. The user should be aware that simply reading the file then performing operations that are dependent on the proper crystallographic and sample reference frame will be undefined or simply **wrong**. In order to bring the crystal reference frame and sample reference frame into coincidence, rotations will need to be applied to the data. +This **Filter** will read a single .ang file into a new **Image Geometry**, allowing the immediate use of **Filters** on the data instead of having to generate the intermediate .h5ebsd file. A **Cell Attribute Matrix** and **Ensemble Attribute Matrix** will also be created to hold the imported EBSD information. Currently, the user has no control over the names of the created **Attribute Arrays**. The user should be aware that simply reading the file then performing operations that are dependent on the proper crystallographic and sample reference frame will be undefined or simply **wrong**. In order to bring the crystal reference frame and sample reference frame into coincidence, rotations will need to be applied to the data. + +### Created Data + +The **Image Geometry** dimensions come from the `NCOLS_EVEN`/`NROWS` header values, the spacing from `XSTEP`/`YSTEP` (Z spacing is 1.0), the origin is (0, 0, 0), and the length units are always **micrometers**. + +| Location | Array | Type | Notes | +|---|---|---|---| +| Cell | EulerAngles | float32 (3) | phi1, PHI, phi2 columns interleaved, in radians as stored in the file | +| Cell | Phases | int32 | Phase values less than 1 (un-indexed points) are remapped to 1 | +| Cell | Image Quality, Confidence Index, SEM Signal, Fit, X Position, Y Position | float32 | Copied verbatim from the file columns | +| Ensemble | CrystalStructures | uint32 | From each phase's TSL `Symmetry` code; index 0 is reserved for the invalid phase (999) | +| Ensemble | MaterialName | string | Whitespace-trimmed material name; index 0 is "Invalid Phase" | +| Ensemble | LatticeConstants | float32 (6) | a, b, c (Angstroms), alpha, beta, gamma (degrees); index 0 is all zeros | + +### Note on Length Units + +The created **Image Geometry** is always marked as **micrometers**, matching standard EDAX SEM-based `.ang` files. Legacy DREAM.3D 6.5 detected certain retired TEM/ACOM `.ang` variants and marked them as nanometers; EDAX retired those file formats over a decade ago and this filter does not special-case them. If you are importing such an archival file, set the geometry units manually after import. ### Default TSL Transformations -If the data has come from a TSL acquisition system and the settings of the acquisition software were in the default modes, he following reference frame transformations may need to be performed based on the version of the OIM Analysis software being used to collect the data: +If the data has come from a TSL acquisition system and the settings of the acquisition software were in the default modes, the following reference frame transformations may need to be performed based on the version of the OIM Analysis software being used to collect the data: + Sample Reference Frame: 180o about the <010> Axis + Crystal Reference Frame: 90o about the <001> Axis @@ -25,7 +42,7 @@ OIMAnalysis can create EBSD data sampled on a hexagonal grid. The user can look # GRID: HexGrid ``` -If the user's .ang files are hexagonal grid files then they will need to run the {ref}`Convert EDAX Hex Grid to Square Grid (.ang)` filter to first convert the input files square gridded files. +If the user's .ang files are hexagonal grid files then they will need to run the {ref}`Convert EDAX Hex Grid to Square Grid (.ang)` filter to first convert the input files to square gridded files. This filter rejects hexagonal grid files during preflight with error `-19500`. ### Note on .ang file Data Ordering @@ -48,10 +65,17 @@ Fit of Solution ## Example Pipelines -+ INL Export -+ Export Small IN100 ODF Data (StatsGenerator) -+ Edax IPF Colors -+ Confidence Index Histogram ++ Read_EDAX_Ang_File ++ Edax_IPF_Colors ++ CI_Histogram ++ EBSD_Hexagonal_Data_Analysis ++ ReplaceElementAttributesWithNeighbor (SimplnxCore) ++ ExtractVertexGeometry (SimplnxCore) ++ ApplyTransformation_Image (SimplnxCore) + +## Differences from DREAM.3D 6.5 + +Behavioral differences from the legacy `ReadAngData` filter (material-name trimming, length-unit handling for retired TEM/ACOM files, error-code changes, and a legacy crash on non-contiguous phase indices) are documented in the source tree at `src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md`. ## License & Copyright diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp index 4b34d4c179..4e3f464d5a 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp @@ -8,9 +8,9 @@ #include -using namespace nx::core; +#include -using FloatVec3Type = std::vector; +using namespace nx::core; // ----------------------------------------------------------------------------- ReadAngData::ReadAngData(DataStructure& dataStructure, const IFilter::MessageHandler& msgHandler, const std::atomic_bool& shouldCancel, ReadAngDataInputValues* inputValues) @@ -27,6 +27,7 @@ ReadAngData::~ReadAngData() noexcept = default; // ----------------------------------------------------------------------------- Result<> ReadAngData::operator()() { + m_MessageHandler(IFilter::Message::Type::Info, fmt::format("Reading .ang file '{}'", m_InputValues->InputFile.string())); ebsdlib::AngReader reader; reader.setFileName(m_InputValues->InputFile.string()); const int32_t err = reader.readFile(); @@ -34,90 +35,96 @@ Result<> ReadAngData::operator()() { return MakeErrorResult(reader.getErrorCode(), reader.getErrorMessage()); } - const auto result = loadMaterialInfo(&reader); - if(result.first < 0) + Result<> result = loadMaterialInfo(&reader); + if(result.invalid()) { - return MakeErrorResult(result.first, result.second); + return result; } if(m_ShouldCancel) { return {}; } + m_MessageHandler(IFilter::Message::Type::Info, "Copying cell data into the Image Geometry"); copyRawEbsdData(&reader); return {}; } // ----------------------------------------------------------------------------- -std::pair ReadAngData::loadMaterialInfo(ebsdlib::AngReader* reader) const +Result<> ReadAngData::loadMaterialInfo(ebsdlib::AngReader* reader) const { - const std::vector phases = reader->getPhaseVector(); if(phases.empty()) { - return {reader->getErrorCode(), reader->getErrorMessage()}; + return MakeErrorResult(reader->getErrorCode(), reader->getErrorMessage()); } - const DataPath CellEnsembleAttributeMatrixPath = m_InputValues->DataContainerName.createChildPath(m_InputValues->CellEnsembleAttributeMatrixName); + const DataPath cellEnsembleAttributeMatrixPath = m_InputValues->DataContainerName.createChildPath(m_InputValues->CellEnsembleAttributeMatrixName); - auto& crystalStructures = m_DataStructure.getDataRefAs(CellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::CrystalStructures)); + auto& crystalStructures = m_DataStructure.getDataRefAs(cellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::CrystalStructures)); - auto& materialNames = m_DataStructure.getDataRefAs(CellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::MaterialName)); + auto& materialNames = m_DataStructure.getDataRefAs(cellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::MaterialName)); - auto& latticeConstants = m_DataStructure.getDataRefAs(CellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::LatticeConstants)); + auto& latticeConstants = m_DataStructure.getDataRefAs(cellEnsembleAttributeMatrixPath.createChildPath(ebsdlib::AngFile::LatticeConstants)); const std::string k_InvalidPhase = "Invalid Phase"; + const usize numTuples = crystalStructures.getNumberOfTuples(); - // Initialize the zero'th element to unknowns. The other elements will - // be filled in based on values from the data file - crystalStructures[0] = ebsdlib::CrystalStructure::UnknownCrystalStructure; - materialNames[0] = k_InvalidPhase; - - for(size_t i = 0; i < 6; i++) + // Initialize EVERY slot to the "Invalid Phase" defaults first. Slot 0 is always the + // invalid phase, and any slot not covered by a phase section in the file (possible when + // the file's phase indices are not contiguous) keeps these defaults instead of + // zero-initialized garbage. + for(usize tupleIndex = 0; tupleIndex < numTuples; tupleIndex++) { - latticeConstants.getDataStoreRef().setComponent(0, i, 0.0F); + crystalStructures[tupleIndex] = ebsdlib::CrystalStructure::UnknownCrystalStructure; + materialNames[tupleIndex] = k_InvalidPhase; + for(usize i = 0; i < 6; i++) + { + latticeConstants.getDataStoreRef().setComponent(tupleIndex, i, 0.0F); + } } for(const ebsdlib::AngPhase::Pointer& phase : phases) { const int32_t phaseID = phase->getPhaseIndex(); + // The ensemble arrays were sized in preflight from the same file's largest phase index. + // This guard only trips if the file changed between preflight and execute. + if(phaseID < 1 || static_cast(phaseID) >= numTuples) + { + return MakeErrorResult(-19502, fmt::format("Phase index {} from the .ang file falls outside the Ensemble Attribute Matrix range [1, {}]. The input file may have changed since preflight.", + phaseID, numTuples - 1)); + } crystalStructures[phaseID] = phase->determineOrientationOpsIndex(); - std::string materialName = phase->getMaterialName(); - materialName = nx::core::StringUtilities::replace(materialName, "MaterialName", ""); - materialName = nx::core::StringUtilities::trimmed(materialName); - materialNames[phaseID] = materialName; + // EbsdLib's AngPhase::parseMaterialName() rejoins the name tokens with a trailing + // space (e.g. "Nickel "), so the name must be trimmed before storing it. + materialNames[phaseID] = nx::core::StringUtilities::trimmed(phase->getMaterialName()); std::vector lattConst = phase->getLatticeConstants(); - for(size_t i = 0; i < 6; i++) + for(usize i = 0; i < 6; i++) { latticeConstants.getDataStoreRef().setComponent(phaseID, i, lattConst[i]); } } - return {0, ""}; + return {}; } // ----------------------------------------------------------------------------- void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const { - const DataPath CellAttributeMatrixPath = m_InputValues->DataContainerName.createChildPath(m_InputValues->CellAttributeMatrixName); - - std::vector cDims = {1}; + const DataPath cellAttributeMatrixPath = m_InputValues->DataContainerName.createChildPath(m_InputValues->CellAttributeMatrixName); const auto& imageGeom = m_DataStructure.getDataRefAs(m_InputValues->DataContainerName); const size_t totalCells = imageGeom.getNumberOfCells(); - // Prepare the Cell Attribute Matrix with the correct number of tuples based on the total Cells being read from the file. - std::vector tDims = {imageGeom.getNumXCells(), imageGeom.getNumYCells(), imageGeom.getNumZCells()}; - // Adjust the values of the 'phase' data to correct for invalid values and assign the read Phase Data into the actual DataArray { if(m_ShouldCancel) { return; } - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::AngFile::Phases)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::AngFile::Phases)); int* phasePtr = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::PhaseData)); for(size_t i = 0; i < totalCells; i++) { @@ -138,9 +145,8 @@ void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const const auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Phi1)); const auto* fComp1 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Phi)); const auto* fComp2 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Phi2)); - cDims[0] = 3; - auto& cellEulerAngles = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::AngFile::EulerAngles)); + auto& cellEulerAngles = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::AngFile::EulerAngles)); for(size_t i = 0; i < totalCells; i++) { cellEulerAngles[3 * i] = fComp0[i]; @@ -153,40 +159,40 @@ void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const { return; } - cDims[0] = 1; + { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::ImageQuality)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::ImageQuality)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::ImageQuality)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::ConfidenceIndex)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::ConfidenceIndex)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::ConfidenceIndex)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::SEMSignal)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::SEMSignal)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::SEMSignal)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Fit)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::Fit)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::Fit)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::XPosition)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::XPosition)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::XPosition)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } { auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::YPosition)); - auto& targetArray = m_DataStructure.getDataRefAs(CellAttributeMatrixPath.createChildPath(ebsdlib::Ang::YPosition)); + auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::YPosition)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } } diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp index c49f43e06e..68a3bd031c 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp @@ -2,10 +2,8 @@ #include "OrientationAnalysis/OrientationAnalysis_export.hpp" -#include "simplnx/DataStructure/DataArray.hpp" #include "simplnx/DataStructure/DataPath.hpp" #include "simplnx/DataStructure/DataStructure.hpp" -#include "simplnx/DataStructure/IDataArray.hpp" #include "simplnx/Filter/IFilter.hpp" #include @@ -21,35 +19,6 @@ struct ORIENTATIONANALYSIS_EXPORT ReadAngDataInputValues std::string CellEnsembleAttributeMatrixName; }; -struct ORIENTATIONANALYSIS_EXPORT Ang_Private_Data -{ - std::array dims = {0, 0, 0}; - std::array resolution = {0.0F, 0.0F, 0.0F}; - std::array origin = {0.0F, 0.0F, 0.0F}; - std::vector phases; - int32_t units = 0; -}; - -/** - * @brief The ReadAngDataPrivate class is a private implementation of the ReadAngData class - */ -class ORIENTATIONANALYSIS_EXPORT ReadAngDataPrivate -{ -public: - ReadAngDataPrivate() = default; - ~ReadAngDataPrivate() = default; - - ReadAngDataPrivate(const ReadAngDataPrivate&) = delete; // Copy Constructor Not Implemented - ReadAngDataPrivate(ReadAngDataPrivate&&) = delete; // Move Constructor Not Implemented - ReadAngDataPrivate& operator=(const ReadAngDataPrivate&) = delete; // Copy Assignment Not Implemented - ReadAngDataPrivate& operator=(ReadAngDataPrivate&&) = delete; // Move Assignment Not Implemented - - Ang_Private_Data m_Data; - - std::string m_InputFile_Cache; - fs::file_time_type m_TimeStamp_Cache; -}; - /** * @class ReadAngData * @brief This filter will read a single .ang file into a new Image Geometry, allowing the immediate use of Filters on the data instead of having to generate the intermediate @@ -75,15 +44,19 @@ class ORIENTATIONANALYSIS_EXPORT ReadAngData const ReadAngDataInputValues* m_InputValues = nullptr; /** - * @brief - * @param reader - * @return Error code. + * @brief Populates the Ensemble Attribute Matrix arrays (CrystalStructures, MaterialName, + * LatticeConstants) from the phase sections parsed out of the .ang header. Every slot is + * first initialized to the "Invalid Phase" defaults, then overwritten per parsed phase. + * @param reader The AngReader that has already successfully read the input file. + * @return Error result if no phases were parsed or a phase index falls outside the ensemble arrays. */ - std::pair loadMaterialInfo(ebsdlib::AngReader* reader) const; + Result<> loadMaterialInfo(ebsdlib::AngReader* reader) const; /** - * @brief - * @param reader + * @brief Copies the per-point data columns from the AngReader into the Cell Attribute Matrix + * arrays: remaps phase values < 1 to 1, interleaves phi1/PHI/phi2 into the 3-component + * EulerAngles array, and copies the remaining columns verbatim. + * @param reader The AngReader that has already successfully read the input file. */ void copyRawEbsdData(ebsdlib::AngReader* reader) const; }; diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.cpp index b62755cac3..3fc5c87cc7 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.cpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.cpp @@ -20,6 +20,7 @@ #include #include +#include #include namespace fs = std::filesystem; @@ -118,10 +119,6 @@ IFilter::PreflightResult ReadAngDataFilter::preflightImpl(const DataStructure& d CreateImageGeometryAction::SpacingType spacing = {reader.getXStep(), reader.getYStep(), 1.0F}; CreateImageGeometryAction::OriginType origin = {0.0F, 0.0F, 0.0F}; - // These variables should be updated with the latest data generated for each variable during preflight. - // These will be returned through the preflightResult variable to the - // user interface. You could make these member variables instead if needed. - EbsdReaderUtilities::GeneratePreflightScanInformation(reader, preflightUpdatedValues); EbsdReaderUtilities::GeneratePreflightPhaseInformation(reader, preflightUpdatedValues); @@ -171,9 +168,17 @@ IFilter::PreflightResult ReadAngDataFilter::preflightImpl(const DataStructure& d resultOutputActions.value().appendAction(std::move(action)); } - // Create the Ensemble AttributeMatrix + // Create the Ensemble AttributeMatrix. Slot 0 is always reserved for the "Invalid Phase", + // and the arrays are sized from the LARGEST phase index in the file (not the phase count) + // because the ensemble arrays are indexed by AngPhase::getPhaseIndex(), which is not + // guaranteed to be contiguous starting at 1. std::vector> angPhases = reader.getPhaseVector(); - tupleDims = {angPhases.size() + 1}; // Always create 1 extra slot for the phases. + size_t maxPhaseIndex = 0; + for(const std::shared_ptr& angPhase : angPhases) + { + maxPhaseIndex = std::max(maxPhaseIndex, static_cast(std::max(angPhase->getPhaseIndex(), 0))); + } + tupleDims = {std::max(maxPhaseIndex, angPhases.size()) + 1}; DataPath ensembleAttributeMatrixPath = pImageGeometryPath.createChildPath(pCellEnsembleAttributeMatrixNameValue); { auto createAttributeMatrixAction = std::make_unique(ensembleAttributeMatrixPath, tupleDims); diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.hpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.hpp index d935aaaf88..4e5a7e6d65 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.hpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/ReadAngDataFilter.hpp @@ -5,13 +5,8 @@ #include "simplnx/Filter/FilterTraits.hpp" #include "simplnx/Filter/IFilter.hpp" -#include - namespace nx::core { -// forward declare the private implementation -class ReadAngDataPrivate; - /** * @class ReadAngDataFilter * @brief This filter will read a single .ang file into a new Image Geometry, allowing the immediate use of Filters on the data instead of having to generate the intermediate @@ -120,9 +115,6 @@ class ORIENTATIONANALYSIS_EXPORT ReadAngDataFilter : public IFilter */ Result<> executeImpl(DataStructure& dataStructure, const Arguments& filterArgs, const PipelineFilter* pipelineNode, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel, const ExecutionContext& executionContext) const override; - -private: - std::shared_ptr m_AngDataPrivate; }; } // namespace nx::core diff --git a/src/Plugins/OrientationAnalysis/test/CMakeLists.txt b/src/Plugins/OrientationAnalysis/test/CMakeLists.txt index 41a4b2821e..12558701a7 100644 --- a/src/Plugins/OrientationAnalysis/test/CMakeLists.txt +++ b/src/Plugins/OrientationAnalysis/test/CMakeLists.txt @@ -148,7 +148,6 @@ if(EXISTS "${DREAM3D_DATA_DIR}" AND SIMPLNX_DOWNLOAD_TEST_FILES) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME INL_writer.tar.gz SHA512 7d723351e51e84540abfbc38e69a6014852ba34808f9d216a27063a616bcfbd5eb708405305fd83334e48c9ca133d3d0be797c05040e4a115cc612e385d9ada6) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME neighbor_orientation_correlation.tar.gz SHA512 122367452174ade2f24dde7a4610bddc4f147a223722d9b30c1df9eaa2cd2bf25e1c7957aba83f3f9de79b4eadd79339b848f9530d1ebf44c69244ea5442cf85) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME Pole_Figure_Exemplars_v6.tar.gz SHA512 23ad8853c38c66f498c7a9c43d6152419506b2bcb6c7e4854728dd3ad81efd309541981bf703ad9b0a1e9b506a0098da918599d5209cb06353281d9cecf39223) - download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME read_ang_test.tar.gz SHA512 de7cd89d925da01f291f44686964ec89d469659d0005219f9869afe26b8f62af278461ac3f5deb3afe7f3e65ec074ab3a1357d77a1a5f92eb3a1ea8cc5e4b236) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME segment_features_test_data.tar.gz SHA512 317d69384330d40c673f8e1a42df003dede5ac85331b2549e9f45467f7af6b74284f8dad1120427690719ebcd5066830e17031533381cc2cc0cc8622a422b914) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME Small_IN100_Ang_Files.tar.gz SHA512 79e9f6948d4e8e06187e11216a67596fa786ffd2700e51f594ad014090383eb8bcc003e14de2e88082aa9ae512cc4fc9cee22c80066fc54f38c3ebc75267eb5b) download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME Small_IN100_dream3d_v3.tar.gz SHA512 bfa9547e787b0f8e8122702da0eb4e519f48a48bf4f94aad020f72479d071d32dfc96a1425705874c68507a61ed391d28606d9c4f4acd559043ef0ace64fd33f) diff --git a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp index 4717f7ee09..9adbbbe99c 100644 --- a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp +++ b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp @@ -1,12 +1,50 @@ +/* ============================================================================ + * ReadAngData V&V test suite. + * + * Verification is established INDEPENDENTLY of legacy DREAM3D, per the V&V + * policy (src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md): + * + * - .ang parsing : Class 2 (EbsdLib reference, trusted & NOT re-tested). + * EbsdLib's AngReader owns header parsing, data-column + * parsing, grid-order fix-up and its own error codes. + * We do not re-test any of that here. + * - SIMPLNX value-add : Class 1 (analytical) + Class 4 (invariant). The + * filter's value-add is deterministic data plumbing on + * top of AngReader: geometry construction from header + * values, per-column array creation, phase<1 -> 1 + * remap, Euler-angle interleave, ensemble slot-0 + * defaults, symmetry -> LaueOps index mapping, + * material-name trimming and lattice-constant copy. + * The toy .ang fixture below is hand-authored and every + * expected value is hand-derived from the fixture text. + * + * All fixture values are exactly representable in float32 (multiples of 1/8) + * so the expected values can be asserted with exact equality. + * + * The prior exemplar archive read_ang_test.tar.gz was a CIRCULAR ORACLE (the + * exemplar .dream3d was generated from this filter's own output) and has been + * retired in favor of the inline oracle below. + * ========================================================================== */ + #include "OrientationAnalysis/Filters/ReadAngDataFilter.hpp" #include "OrientationAnalysis/OrientationAnalysis_test_dirs.hpp" +#include "simplnx/Core/Application.hpp" +#include "simplnx/DataStructure/AttributeMatrix.hpp" +#include "simplnx/DataStructure/DataArray.hpp" +#include "simplnx/DataStructure/Geometry/ImageGeom.hpp" +#include "simplnx/DataStructure/StringArray.hpp" #include "simplnx/Parameters/FileSystemPathParameter.hpp" +#include "simplnx/Pipeline/Pipeline.hpp" +#include "simplnx/Pipeline/PipelineFilter.hpp" #include "simplnx/UnitTest/UnitTestCommon.hpp" #include #include +#include +#include +#include namespace fs = std::filesystem; @@ -14,96 +52,385 @@ using namespace nx::core; using namespace nx::core::Constants; using namespace nx::core::UnitTest; -TEST_CASE("OrientationAnalysis::ReadAngData: Exemplary Test", "[OrientationAnalysis][ReadAngDataFilter]") +namespace { - UnitTest::LoadPlugins(); +//------------------------------------------------------------------------------ +// Toy .ang fixture: 3 columns x 2 rows, square grid, XSTEP 0.25, YSTEP 0.5, +// two phases (Cubic Nickel, Hexagonal Titanium). Data columns are +// phi1 PHI phi2 x y IQ CI Phase SEM Fit +// Points 2 and 5 carry Phase 0 to exercise the filter's phase<1 -> 1 remap. +//------------------------------------------------------------------------------ +const std::string k_HeaderPrefix = R"(# TEM_PIXperUM 1.000000 +# x-star 0.500000 +# y-star 0.500000 +# z-star 0.500000 +# WorkingDistance 15.000000 +# +)"; - const nx::core::UnitTest::TestFileSentinel testDataSentinel(nx::core::unit_test::k_TestFilesDir, "read_ang_test.tar.gz", "read_ang_test"); +const std::string k_PhaseBlock = R"(# Phase 1 +# MaterialName Nickel +# Formula Ni +# Info +# Symmetry 43 +# LatticeConstants 3.520 3.520 3.520 90.000 90.000 90.000 +# NumberFamilies 1 +# hklFamilies 1 1 1 1 8.469246 1 +# Categories 0 0 0 0 0 +# +# Phase 2 +# MaterialName Titanium (Alpha) +# Formula Ti +# Info +# Symmetry 62 +# LatticeConstants 2.950 2.950 4.680 90.000 90.000 120.000 +# NumberFamilies 1 +# hklFamilies 1 0 0 1 1.000000 1 +# Categories 0 0 0 0 0 +# +)"; - // Read Exemplar DREAM3D File - auto exemplarFilePath = fs::path(fmt::format("{}/read_ang_test/read_ang_test.dream3d", unit_test::k_TestFilesDir)); - DataStructure exemplarDataStructure = LoadDataStructure(exemplarFilePath); +// Only the second phase section of k_PhaseBlock: a legal-but-unusual file whose +// phase indices do not start at 1. Exercises the non-contiguous-phase sizing. +const std::string k_SparsePhaseBlock = R"(# Phase 2 +# MaterialName Titanium (Alpha) +# Formula Ti +# Info +# Symmetry 62 +# LatticeConstants 2.950 2.950 4.680 90.000 90.000 120.000 +# NumberFamilies 1 +# hklFamilies 1 0 0 1 1.000000 1 +# Categories 0 0 0 0 0 +# +)"; - // Instantiate the filter, a DataStructure object and an Arguments Object - ReadAngDataFilter filter; - DataStructure dataStructure; - Arguments args; +// {} is the GRID line (or empty to omit it entirely) +const std::string k_GridBlockFmt = R"({}# XSTEP: 0.250000 +# YSTEP: 0.500000 +# NCOLS_ODD: 3 +# NCOLS_EVEN: 3 +# NROWS: 2 +# +# OPERATOR: VVOracle +# +# SAMPLEID: +# +# SCANID: +# +)"; + +const std::string k_DataBlock = R"( 0.12500 0.25000 0.37500 0.00000 0.00000 10.5000 0.500 1 100.000 0.750 + 0.50000 0.62500 0.75000 0.25000 0.00000 20.2500 0.250 2 200.000 1.500 + 0.87500 1.00000 1.12500 0.50000 0.00000 30.7500 0.125 0 300.000 2.250 + 1.25000 1.37500 1.50000 0.00000 0.50000 40.5000 0.375 1 400.000 3.000 + 1.62500 1.75000 1.87500 0.25000 0.50000 50.2500 0.625 2 500.000 3.750 + 2.00000 2.12500 2.25000 0.50000 0.50000 60.7500 0.875 0 600.000 4.500 +)"; - const fs::path inputAngFile(fmt::format("{}/read_ang_test/read_ang_test.ang", unit_test::k_TestFilesDir)); +//------------------------------------------------------------------------------ +std::string SqrGridBlock() +{ + return fmt::format(fmt::runtime(k_GridBlockFmt), "# GRID: SqrGrid\n"); +} + +//------------------------------------------------------------------------------ +fs::path WriteAngFile(const std::string& fileName, const std::string& contents) +{ + const fs::path filePath = fs::path(unit_test::k_BinaryTestOutputDir.view()) / fileName; + std::ofstream out(filePath, std::ios::out | std::ios::trunc); + REQUIRE(out.is_open()); + out << contents; + out.close(); + return filePath; +} - // Create default Parameters for the filter. +//------------------------------------------------------------------------------ +Arguments MakeDefaultArgs(const fs::path& inputAngFile) +{ + Arguments args; args.insertOrAssign(ReadAngDataFilter::k_InputFile_Key, std::make_any(inputAngFile)); args.insertOrAssign(ReadAngDataFilter::k_CreatedImageGeometryPath_Key, std::make_any(k_DataContainerPath)); args.insertOrAssign(ReadAngDataFilter::k_CellAttributeMatrixName_Key, std::make_any(k_CellData)); args.insertOrAssign(ReadAngDataFilter::k_CellEnsembleAttributeMatrixName_Key, std::make_any(k_EnsembleAttributeMatrix)); + return args; +} + +//------------------------------------------------------------------------------ +template +void CompareArrayValues(const DataStructure& dataStructure, const DataPath& arrayPath, const std::vector& expected) +{ + REQUIRE_NOTHROW(dataStructure.getDataRefAs>(arrayPath)); + const auto& dataArrayRef = dataStructure.getDataRefAs>(arrayPath); + REQUIRE(dataArrayRef.getSize() == expected.size()); + for(usize i = 0; i < expected.size(); i++) + { + INFO(fmt::format("Array '{}' index {}", arrayPath.toString(), i)); + REQUIRE(dataArrayRef[i] == expected[i]); + } +} +} // namespace + +//------------------------------------------------------------------------------ +// Class 1 (analytical) + Class 4 (invariant) oracle. Every expected value below +// is hand-derived from the fixture text at the top of this file; none of it was +// produced by running DREAM3D (any version). +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: Class 1 Analytical Oracle", "[OrientationAnalysis][ReadAngDataFilter]") +{ + UnitTest::LoadPlugins(); + + const fs::path inputAngFile = WriteAngFile("read_ang_vv_oracle.ang", k_HeaderPrefix + k_PhaseBlock + SqrGridBlock() + k_DataBlock); + + ReadAngDataFilter filter; + DataStructure dataStructure; + Arguments args = MakeDefaultArgs(inputAngFile); - // Preflight the filter and check result auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); - // Execute the filter and check the result auto executeResult = filter.execute(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result); - CompareExemplarToGeneratedData(dataStructure, exemplarDataStructure, k_CellAttributeMatrix, k_ExemplarDataContainer); + const DataPath cellAMPath = k_DataContainerPath.createChildPath(k_CellData); + const DataPath ensembleAMPath = k_DataContainerPath.createChildPath(k_EnsembleAttributeMatrix); + + // --- Geometry (Class 1: dims/spacing straight from the header; origin and + // --- z-extent are the filter's hard-coded value-add) ----------------------- + REQUIRE_NOTHROW(dataStructure.getDataRefAs(k_DataContainerPath)); + const auto& imageGeom = dataStructure.getDataRefAs(k_DataContainerPath); + { + const SizeVec3 dims = imageGeom.getDimensions(); // NCOLS_EVEN=3, NROWS=2, one slice + REQUIRE(dims == SizeVec3(3, 2, 1)); + const FloatVec3 spacing = imageGeom.getSpacing(); // XSTEP=0.25, YSTEP=0.5, z hard-coded 1.0 + REQUIRE(spacing == FloatVec3(0.25F, 0.5F, 1.0F)); + const FloatVec3 origin = imageGeom.getOrigin(); // hard-coded (0,0,0) + REQUIRE(origin == FloatVec3(0.0F, 0.0F, 0.0F)); + REQUIRE(imageGeom.getUnits() == IGeometry::LengthUnit::Micrometer); + } + + // --- Cell data (Class 1) --------------------------------------------------- + // Phases: file column 8 is {1,2,0,1,2,0}; the filter remaps phase<1 -> 1. + CompareArrayValues(dataStructure, cellAMPath.createChildPath("Phases"), {1, 2, 1, 1, 2, 1}); + + // EulerAngles: file columns 1-3 interleaved per point (phi1_i, PHI_i, phi2_i). + CompareArrayValues(dataStructure, cellAMPath.createChildPath("EulerAngles"), + {0.125F, 0.25F, 0.375F, 0.5F, 0.625F, 0.75F, 0.875F, 1.0F, 1.125F, 1.25F, 1.375F, 1.5F, 1.625F, 1.75F, 1.875F, 2.0F, 2.125F, 2.25F}); + + // Pass-through columns, copied verbatim from the file. + CompareArrayValues(dataStructure, cellAMPath.createChildPath("Image Quality"), {10.5F, 20.25F, 30.75F, 40.5F, 50.25F, 60.75F}); + CompareArrayValues(dataStructure, cellAMPath.createChildPath("Confidence Index"), {0.5F, 0.25F, 0.125F, 0.375F, 0.625F, 0.875F}); + CompareArrayValues(dataStructure, cellAMPath.createChildPath("SEM Signal"), {100.0F, 200.0F, 300.0F, 400.0F, 500.0F, 600.0F}); + CompareArrayValues(dataStructure, cellAMPath.createChildPath("Fit"), {0.75F, 1.5F, 2.25F, 3.0F, 3.75F, 4.5F}); + CompareArrayValues(dataStructure, cellAMPath.createChildPath("X Position"), {0.0F, 0.25F, 0.5F, 0.0F, 0.25F, 0.5F}); + CompareArrayValues(dataStructure, cellAMPath.createChildPath("Y Position"), {0.0F, 0.0F, 0.0F, 0.5F, 0.5F, 0.5F}); + + // --- Ensemble data ---------------------------------------------------------- + // Class 4 invariant: tuple count is always (number of phases in file) + 1. + REQUIRE_NOTHROW(dataStructure.getDataRefAs(ensembleAMPath)); + const auto& ensembleAM = dataStructure.getDataRefAs(ensembleAMPath); + REQUIRE(ensembleAM.getShape() == std::vector{3}); + + // CrystalStructures (Class 1): slot 0 is the UnknownCrystalStructure default + // (999); TSL Symmetry 43 (Oh, m-3m) -> Cubic_High (1); TSL Symmetry 62 + // (D6h, 6/mmm) -> Hexagonal_High (0). + CompareArrayValues(dataStructure, ensembleAMPath.createChildPath("CrystalStructures"), {999, 1, 0}); + + // LatticeConstants (Class 1): slot 0 zero-filled default; slots 1/2 copied + // from the LatticeConstants header lines (a, b, c, alpha, beta, gamma). + CompareArrayValues(dataStructure, ensembleAMPath.createChildPath("LatticeConstants"), + {0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 3.52F, 3.52F, 3.52F, 90.0F, 90.0F, 90.0F, 2.95F, 2.95F, 4.68F, 90.0F, 90.0F, 120.0F}); + + // MaterialName (Class 1): slot 0 is the "Invalid Phase" default; EbsdLib's + // AngPhase::parseMaterialName() rejoins tokens with a TRAILING space + // ("Nickel ", "Titanium (Alpha) ") and the filter's value-add trims it. + { + const DataPath materialNamePath = ensembleAMPath.createChildPath("MaterialName"); + REQUIRE_NOTHROW(dataStructure.getDataRefAs(materialNamePath)); + const auto& materialNamesRef = dataStructure.getDataRefAs(materialNamePath); + REQUIRE(materialNamesRef.getNumberOfTuples() == 3); + REQUIRE(materialNamesRef[0] == "Invalid Phase"); + REQUIRE(materialNamesRef[1] == "Nickel"); + REQUIRE(materialNamesRef[2] == "Titanium (Alpha)"); + } UnitTest::CheckArraysInheritTupleDims(dataStructure); } -TEST_CASE("OrientationAnalysis::ReadAngData: Invalid Phase", "[OrientationAnalysis][ReadAngDataFilter]") +//------------------------------------------------------------------------------ +// Class 4 invariant: a file whose phase indices do NOT start at 1 (only a +// "# Phase 2" section) must still produce ensemble arrays sized to +// maxPhaseIndex + 1, with every slot not covered by a phase section holding +// the "Invalid Phase" defaults. Regression pin for the out-of-bounds ensemble +// write found during the V&V algorithm review. +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: Non-Contiguous Phase Index", "[OrientationAnalysis][ReadAngDataFilter]") { UnitTest::LoadPlugins(); - const nx::core::UnitTest::TestFileSentinel testDataSentinel(nx::core::unit_test::k_TestFilesDir, "read_ang_test.tar.gz", "read_ang_test"); + const fs::path inputAngFile = WriteAngFile("read_ang_vv_sparse_phase.ang", k_HeaderPrefix + k_SparsePhaseBlock + SqrGridBlock() + k_DataBlock); - // Instantiate the filter, a DataStructure object and an Arguments Object ReadAngDataFilter filter; DataStructure dataStructure; - Arguments args; + Arguments args = MakeDefaultArgs(inputAngFile); - const fs::path inputAngFile(fmt::format("{}/read_ang_test/ang_unit_test_invalid_phase.ang", unit_test::k_TestFilesDir)); + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); - // Create default Parameters for the filter. - args.insertOrAssign(ReadAngDataFilter::k_InputFile_Key, std::make_any(inputAngFile)); - args.insertOrAssign(ReadAngDataFilter::k_CreatedImageGeometryPath_Key, std::make_any(k_DataContainerPath)); - args.insertOrAssign(ReadAngDataFilter::k_CellAttributeMatrixName_Key, std::make_any(k_CellData)); - args.insertOrAssign(ReadAngDataFilter::k_CellEnsembleAttributeMatrixName_Key, std::make_any(k_EnsembleAttributeMatrix)); + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result); + + const DataPath ensembleAMPath = k_DataContainerPath.createChildPath(k_EnsembleAttributeMatrix); + + // maxPhaseIndex = 2 -> 3 tuples, even though the file defines only one phase. + REQUIRE_NOTHROW(dataStructure.getDataRefAs(ensembleAMPath)); + const auto& ensembleAM = dataStructure.getDataRefAs(ensembleAMPath); + REQUIRE(ensembleAM.getShape() == std::vector{3}); + + // Slot 1 (no phase section in the file) keeps the "Invalid Phase" defaults. + CompareArrayValues(dataStructure, ensembleAMPath.createChildPath("CrystalStructures"), {999, 999, 0}); + CompareArrayValues(dataStructure, ensembleAMPath.createChildPath("LatticeConstants"), + {0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 0.0F, 2.95F, 2.95F, 4.68F, 90.0F, 90.0F, 120.0F}); + { + const DataPath materialNamePath = ensembleAMPath.createChildPath("MaterialName"); + REQUIRE_NOTHROW(dataStructure.getDataRefAs(materialNamePath)); + const auto& materialNamesRef = dataStructure.getDataRefAs(materialNamePath); + REQUIRE(materialNamesRef[0] == "Invalid Phase"); + REQUIRE(materialNamesRef[1] == "Invalid Phase"); + REQUIRE(materialNamesRef[2] == "Titanium (Alpha)"); + } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} + +//------------------------------------------------------------------------------ +// Value-add error path: the filter refuses HexGrid files at preflight (-19500). +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: HexGrid Preflight Error (-19500)", "[OrientationAnalysis][ReadAngDataFilter]") +{ + UnitTest::LoadPlugins(); + + const std::string hexGridBlock = fmt::format(fmt::runtime(k_GridBlockFmt), "# GRID: HexGrid\n"); + const fs::path inputAngFile = WriteAngFile("read_ang_vv_hexgrid.ang", k_HeaderPrefix + k_PhaseBlock + hexGridBlock + k_DataBlock); + + ReadAngDataFilter filter; + DataStructure dataStructure; + Arguments args = MakeDefaultArgs(inputAngFile); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); + REQUIRE(preflightResult.outputActions.errors()[0].code == -19500); +} + +//------------------------------------------------------------------------------ +// Value-add error path: a file without a GRID header key fails preflight (-19501). +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: Missing GRID Preflight Error (-19501)", "[OrientationAnalysis][ReadAngDataFilter]") +{ + UnitTest::LoadPlugins(); + + const std::string noGridBlock = fmt::format(fmt::runtime(k_GridBlockFmt), ""); + const fs::path inputAngFile = WriteAngFile("read_ang_vv_missing_grid.ang", k_HeaderPrefix + k_PhaseBlock + noGridBlock + k_DataBlock); + + ReadAngDataFilter filter; + DataStructure dataStructure; + Arguments args = MakeDefaultArgs(inputAngFile); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); + REQUIRE(preflightResult.outputActions.errors()[0].code == -19501); +} + +//------------------------------------------------------------------------------ +// EbsdLib error passthrough: a header with no Phase sections preflights fine +// (header parse succeeds) but execute fails with AngReader's -150. +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - No Phase (-150)", "[OrientationAnalysis][ReadAngDataFilter]") +{ + UnitTest::LoadPlugins(); + + const fs::path inputAngFile = WriteAngFile("read_ang_vv_no_phase.ang", k_HeaderPrefix + SqrGridBlock() + k_DataBlock); + + ReadAngDataFilter filter; + DataStructure dataStructure; + Arguments args = MakeDefaultArgs(inputAngFile); - // Preflight the filter and check result auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); - // Execute the filter and check the result auto executeResult = filter.execute(dataStructure, args); SIMPLNX_RESULT_REQUIRE_INVALID(executeResult.result); REQUIRE(executeResult.result.errors()[0].code == -150); } -TEST_CASE("OrientationAnalysis::ReadAngData: Invalid Columns & Rows", "[OrientationAnalysis][ReadAngDataFilter]") +//------------------------------------------------------------------------------ +// EbsdLib error passthrough: the header declares 3x2 points but the data block +// is truncated to 4 lines; AngReader reports -600 at execute. +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - Truncated Data (-600)", "[OrientationAnalysis][ReadAngDataFilter]") { UnitTest::LoadPlugins(); - const nx::core::UnitTest::TestFileSentinel testDataSentinel(nx::core::unit_test::k_TestFilesDir, "read_ang_test.tar.gz", "read_ang_test"); + // Keep only the first 4 of 6 data lines. + std::string truncatedData = k_DataBlock; + for(int i = 0; i < 2; i++) + { + truncatedData.erase(truncatedData.find_last_of('\n', truncatedData.size() - 2) + 1); + } + const fs::path inputAngFile = WriteAngFile("read_ang_vv_truncated.ang", k_HeaderPrefix + k_PhaseBlock + SqrGridBlock() + truncatedData); - // Instantiate the filter, a DataStructure object and an Arguments Object ReadAngDataFilter filter; DataStructure dataStructure; - Arguments args; - - const fs::path inputAngFile(fmt::format("{}/read_ang_test/ang_unit_test_invalid_cols_rows.ang", unit_test::k_TestFilesDir)); - - // Create default Parameters for the filter. - args.insertOrAssign(ReadAngDataFilter::k_InputFile_Key, std::make_any(inputAngFile)); - args.insertOrAssign(ReadAngDataFilter::k_CreatedImageGeometryPath_Key, std::make_any(k_DataContainerPath)); - args.insertOrAssign(ReadAngDataFilter::k_CellAttributeMatrixName_Key, std::make_any(k_CellData)); - args.insertOrAssign(ReadAngDataFilter::k_CellEnsembleAttributeMatrixName_Key, std::make_any(k_EnsembleAttributeMatrix)); + Arguments args = MakeDefaultArgs(inputAngFile); - // Preflight the filter and check result auto preflightResult = filter.preflight(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); - // Execute the filter and check the result auto executeResult = filter.execute(dataStructure, args); SIMPLNX_RESULT_REQUIRE_INVALID(executeResult.result); REQUIRE(executeResult.result.errors()[0].code == -600); -} \ No newline at end of file +} + +//------------------------------------------------------------------------------ +// SIMPL Backwards Compatibility — validates UUID + parameter conversion from +// the legacy ReadAngData (SIMPL UUID b8e128a8-c2a3-5e6c-a7ad-e4fb864e5d40). +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: SIMPL Backwards Compatibility", "[OrientationAnalysis][ReadAngDataFilter][BackwardsCompatibility]") +{ + auto app = Application::GetOrCreateInstance(); + UnitTest::LoadPlugins(); + auto filterList = app->getFilterList(); + + const fs::path conversionDir = fs::path(nx::core::unit_test::k_SourceDir.view()) / "test" / "simpl_conversion"; + + const std::vector> fixtures = { + {"SIMPL 6.5 (UUID)", conversionDir / "6_5" / "ReadAngDataFilter.json"}, + {"SIMPL 6.4 (Filter_Name)", conversionDir / "6_4" / "ReadAngDataFilter.json"}, + }; + + for(const auto& [label, fixturePath] : fixtures) + { + DYNAMIC_SECTION(label) + { + auto pipelineResult = Pipeline::FromSIMPLFile(fixturePath, filterList); + REQUIRE(pipelineResult.valid()); + + auto& pipeline = pipelineResult.value(); + REQUIRE(pipeline.size() == 1); + + auto* pipelineFilter = dynamic_cast(pipeline.at(0)); + REQUIRE(pipelineFilter != nullptr); + + const IFilter* filter = pipelineFilter->getFilter(); + REQUIRE(filter != nullptr); + REQUIRE(filter->uuid() == FilterTraits::uuid); + + CHECK(pipelineFilter->getComments().empty()); + + const Arguments args = pipelineFilter->getArguments(); + CHECK(args.value(ReadAngDataFilter::k_InputFile_Key) == fs::path("/some/test/path/data.ang")); + CHECK(args.value(ReadAngDataFilter::k_CreatedImageGeometryPath_Key) == DataPath({"ImageDataContainer"})); + CHECK(args.value(ReadAngDataFilter::k_CellAttributeMatrixName_Key) == "CellData"); + CHECK(args.value(ReadAngDataFilter::k_CellEnsembleAttributeMatrixName_Key) == "CellEnsembleData"); + } + } +} diff --git a/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md new file mode 100644 index 0000000000..083f5a72f9 --- /dev/null +++ b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md @@ -0,0 +1,130 @@ +# V&V Report: ReadAngDataFilter + +| | | +|--------|--------------| +| Plugin | OrientationAnalysis | +| SIMPLNX UUID | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| SIMPLNX Human Name | Read EDAX EBSD Data (.ang) | +| DREAM3D 6.5.171 equivalent | `ReadAngData` (SIMPL UUID `b8e128a8-c2a3-5e6c-a7ad-e4fb864e5d40`) — `Source/Plugins/OrientationAnalysis/OrientationAnalysisFilters/ReadAngData.{h,cpp}` | +| Verified commit | ** | +| Status | READY FOR REVIEW — all V&V phases complete; verified-correct against independent oracle; legacy comparison bit-identical on numerics. Pending: sign-off. | +| Sign-off | *Michael Jackson — in progress, 2026-07-07* | + +## At a glance + +| Aspect | Current state | +|------------------------|---------------| +| Algorithm Relationship | **Minor changes.** Faithful port of legacy `ReadAngData` control flow with 4 deliberate deltas: material-name trim added (D1), TEM/ACOM Nanometer-units detection dropped (D2, obsolete file variants), non-contiguous phase-index handling fixed (D3, legacy crashes), and error-code renumbering (D4). The legacy PIMPL file-cache was dropped (no output effect). | +| Oracle (confirmed) | **Confirmed.** **Class 1 (analytical) + Class 4 (invariant)**, scoped to the filter's value-add per the "don't re-test upstream" rule — EbsdLib (vcpkg 3.0.0) owns `.ang` parsing and is trusted (Class 2 boundary). A hand-authored inline toy `.ang` (3×2 grid, 2 phases, phase-0 points, all values float32-exact) with every expected value hand-derived from the fixture text. Encoded as 7 TEST_CASEs in `test/ReadAngDataTest.cpp`; all pass. SIMPLNX matched the oracle on every fixture with zero discrepancies. | +| Code paths enumerated | 13 of 15 paths exercised (see Code path coverage); the two gaps are cancel-signal paths and the file-changed-between-preflight-and-execute guard, both untestable without injection. | +| Tests today | 7 test cases: Class 1+4 analytical oracle, non-contiguous phase-index invariant (regression pin for D3), 2 value-add preflight error paths (-19500/-19501), 2 EbsdLib error passthroughs (-150/-600), and SIMPL 6.4/6.5 backwards-compat (DYNAMIC_SECTION, new — the filter previously had no conversion test). All inline hand-built fixtures — no exemplar archive. | +| Exemplar archive | **None — retired `read_ang_test.tar.gz`** (circular oracle: the exemplar `.dream3d` was generated from this filter's own output). `download_test_data()` entry removed from `test/CMakeLists.txt`; retirement documented in `vv/provenance/read_ang_test.md`. | +| Legacy comparison | **Run (2026-07-07) vs the official DREAM3D 6.5.171 release.** Three fixtures: hand-authored toy, Small IN100 `Slice_1.ang` (189×201 production scan), non-contiguous-phase toy. On the two supported-format fixtures **all numeric outputs are bit-identical** (cell arrays, ensemble arrays, geometry). Differences: MaterialName trailing space (D1) and the non-contiguous-phase fixture, where **6.5.171 segfaults** (D3). | +| Bug flags | One **legacy** bug, empirically confirmed: `ReadAngDataFilter-D3` (6.5.171 out-of-bounds ensemble write → SIGSEGV on non-contiguous phase indices; SIMPLNX fixed during this pass and pinned by test). **No SIMPLNX bugs** — the same latent OOB existed in the NX port and was found by the algorithm review and fixed before the comparison. | +| V&V phase | Discovery, oracle, reconciliation, algorithm review (fixes applied), tests, legacy comparison, deviations, provenance, docs — **complete**. In-core build/tests pass (`simplnx-rel`); OOC build skipped (no OOC build configured in this workspace, per maintainer precedent). Outstanding: sign-off. | + +## Summary + +`ReadAngDataFilter` ("Read EDAX EBSD Data (.ang)") imports a single EDAX TSL `.ang` file into a new Image Geometry: it builds the geometry from the header (dims/step, z=1, origin 0, Micrometer), creates one cell array per data column plus the condensed 3-component `EulerAngles` and remapped `Phases` arrays, and populates the ensemble arrays (`CrystalStructures` via EbsdLib's symmetry mapping, trimmed `MaterialName`, `LatticeConstants`) with slot 0 reserved for the "Invalid Phase". Verification is Class 1 analytical + Class 4 invariant on a hand-authored inline toy `.ang` whose expected outputs were fully hand-derived (EbsdLib parsing itself is trusted upstream and not re-tested). Headline result: SIMPLNX matches the oracle exactly; against DREAM3D 6.5.171 all numeric outputs are bit-identical on supported files, with 4 documented deviations — including one empirically confirmed legacy crash bug (D3) whose latent NX twin was found and fixed during this pass. All 7 unit tests pass; the circular-oracle exemplar archive `read_ang_test.tar.gz` is retired. + +## Algorithm Relationship + +*Classification:* **Minor changes.** + +*Evidence:* Same SIMPL UUID inherited (`b8e128a8-…` → SIMPLNX `5b062816-…`) with `FromSIMPLJson` conversion and 6.4/6.5 fixtures at `test/simpl_conversion/{6_4,6_5}/ReadAngDataFilter.json`. The legacy `ReadAngData::copyRawEbsdData()` (705-line `.cpp`) is preserved essentially line-for-line in `Algorithms/ReadAngData.cpp` (phase remap loop, Euler interleave, verbatim column copies), and `loadMaterialInfo()` keeps the same slot-0-defaults + per-phase-fill structure. + +### Port-time deltas (each mapped to a Deviation entry where user-visible) + +1. **Material-name trim added** (D1). NX applies `StringUtilities::trimmed()`; legacy stores EbsdLib's raw token-rejoin which carries a trailing space. Changes output (string arrays only). +2. **TEM/ACOM units detection dropped** (D2). Legacy scans the header for `# TEM data` / ACOM markers and sets Nanometer units; NX hard-codes Micrometer. Deliberately not restored — EDAX retired those file variants 10+ years ago. Changes output metadata only for obsolete files. +3. **Non-contiguous phase-index handling** (D3, fixed this pass). Preflight now sizes ensemble arrays by `maxPhaseIndex + 1` (identical to `phases.size() + 1` for well-formed files), `loadMaterialInfo` initializes every slot to Invalid-Phase defaults and guards the write range (`-19502`). Legacy has an out-of-bounds write here (segfault, demonstrated). +4. **`determineLaueGroup()` → `determineOrientationOpsIndex()`** — the EbsdLib symmetry→structure mapping was diffed function-body-for-function-body: **byte-identical** (pure rename). No output effect. +5. **Error-code renumbering** (D4) and framework-level file validation (extension/existence moved to `FileSystemPathParameter`). Rejection paths only. +6. **Legacy PIMPL file-cache dropped** (`ReadAngDataPrivate`/`Ang_Private_Data` — ported as dead declarations, removed during this pass). No output effect; NX re-reads the file on each execute. + +*Material PRs since baseline:* none identified for this filter beyond cross-cutting EbsdLib version bumps (the `ebsdlib` namespace migration visible in the current source); the May-2026 touch (`FromSIMPLJson`/conversion fixture refresh) does not alter the algorithm. + +## Oracle + +*Class:* **1 (Analytical) + 4 (Invariant)**; EbsdLib parsing = **Class 2 boundary (trusted, not re-tested)**. + +### The EbsdLib boundary (what we do NOT re-test) + +EbsdLib's `AngReader` owns: header parsing (keys, phase sections, colon-chopping), data-column parsing, square-grid order fix-up, hex-grid rejection at read time, and its own error codes (`-150` no phases, `-600` truncated data, etc.). Those behaviors are upstream's to verify. The filter's value-add — everything this oracle covers — is the deterministic plumbing on top: geometry construction, array creation/typing, phase `<1 → 1` remap, Euler interleave, ensemble sizing + slot-0 defaults, symmetry-index placement, material-name trim, lattice-constant copy, and the value-add error paths (`-19500` HexGrid at preflight, `-19501` missing GRID, `-19502` phase-index range). + +### Applied + +A hand-authored toy `.ang` (3 cols × 2 rows, XSTEP 0.25 / YSTEP 0.5, Cubic Nickel Symmetry 43 + Hexagonal "Titanium (Alpha)" Symmetry 62, points 2 and 5 carrying Phase 0) lives as a string literal in the test source; every fixture value is a multiple of 1/8 so all float32 comparisons are exact. Expected outputs — geometry (3,2,1)/(0.25,0.5,1.0)/(0,0,0)/Micrometer, `Phases {1,2,1,1,2,1}` (remap), the 18-value Euler interleave, 6 verbatim pass-through columns, `CrystalStructures {999,1,0}` (Unknown / Cubic_High / Hexagonal_High), trimmed material names, lattice constants — were derived by hand from the fixture text and the documented TSL symmetry codes, never by running any DREAM3D version. Class 4 invariants: ensemble tuple count = maxPhaseIndex+1, slot-0 (and any uncovered slot's) Invalid-Phase defaults. + +*Encoded:* +- `test/ReadAngDataTest.cpp::"OrientationAnalysis::ReadAngDataFilter: Class 1 Analytical Oracle"` — the full Class 1 + Class 4 assertion set (geometry, 8 cell arrays element-wise, 3 ensemble arrays element-wise). +- `…::"Non-Contiguous Phase Index"` — Class 4 sizing/defaults invariant on the sparse-phase fixture (regression pin for D3). +- `…::"HexGrid Preflight Error (-19500)"`, `…::"Missing GRID Preflight Error (-19501)"` — value-add error paths. +- `…::"EbsdLib Error Passthrough - No Phase (-150)"`, `…::"EbsdLib Error Passthrough - Truncated Data (-600)"` — error propagation from the trusted boundary. +- `…::"SIMPL Backwards Compatibility"` — UUID + argument conversion (6.4 and 6.5 fixtures). + +All 7 pass in the in-core `simplnx-rel` build. Reconciliation found zero SIMPLNX-vs-oracle discrepancies. + +*Second-engineer review:* skipped — documented reason: the filter's value-add is pure data plumbing with no numerical algorithm content; every oracle value is mechanically derivable from the fixture text (grid math, verbatim copies, a documented enum mapping), leaving no design freedom for the author-bias failure mode the review guards against. See `vv/provenance/read_ang_test.md`. + +## Algorithm review + +Line-by-line review performed via the `review-algorithm` skill after oracle reconciliation. All findings applied (all 7 tests still pass after rebuild): + +- **Robustness (Critical):** fixed the latent out-of-bounds ensemble write on non-contiguous phase indices (preflight `maxPhaseIndex+1` sizing; full default initialization of every slot; `-19502` range guard in `loadMaterialInfo`). Legacy twin of this bug segfaults (Deviation D3). +- **Dead code:** removed the vestigial legacy PIMPL (`ReadAngDataPrivate`, `Ang_Private_Data`, `m_AngDataPrivate` member, forward declaration), the never-firing `replace("MaterialName", "")`, unused `tDims`/`cDims` locals, the unused `FloatVec3Type` alias, and scaffold-generator boilerplate comments. +- **API consistency:** `loadMaterialInfo` now returns `Result<>` instead of `std::pair`. +- **Progress messaging:** added status messages before the EbsdLib read and the cell-data copy (the loops themselves are memcpy-speed; no throttled messenger warranted). +- **Naming:** local `CamelCase` path variables renamed to `camelBack`; doc `@brief`s filled in. + +## Code path coverage + +*13 of 15 paths exercised. Source: `src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp` (198 lines) + preflight in `Filters/ReadAngDataFilter.cpp` (~265 lines).* Logical phases: **(a)** preflight (header-only read → output actions), **(b)** execute read + ensemble population (`loadMaterialInfo`), **(c)** cell-data copy (`copyRawEbsdData`). + +| # | Phase | Path | Test case | +|----|-------|------|-----------| +| 1 | (a) Preflight | `readHeaderOnly` error → passthrough error | *Not directly tested as a distinct case — `FileSystemPathParameter` rejects missing files first; unreadable-header content surfaces via the same return as paths 2/3.* Exercised implicitly by every error fixture. | +| 2 | (a) Preflight | `getGrid()` empty → `-19501` | `Missing GRID Preflight Error (-19501)` | +| 3 | (a) Preflight | `getGrid() == "HexGrid"` → `-19500` (after actions are built, so scan info still reaches the UI) | `HexGrid Preflight Error (-19500)` | +| 4 | (a) Preflight | Geometry action: dims (X,Y,1), spacing (XStEP,YSTEP,1), origin (0,0,0), Micrometer | `Class 1 Analytical Oracle` | +| 5 | (a) Preflight | Per-column array creation with int32/float32 type dispatch from `AngFields` | `Class 1 Analytical Oracle` (all 8 cell arrays exist with right types) | +| 6 | (a) Preflight | Ensemble AM sized `maxPhaseIndex + 1`; CrystalStructures/LatticeConstants/MaterialName actions | `Class 1 Analytical Oracle` (3 tuples) + `Non-Contiguous Phase Index` (3 tuples from max index 2) | +| 7 | (b) Execute | `readFile()` error → passthrough | `EbsdLib Error Passthrough - No Phase (-150)` *(−110/−100 variants are the same return statement)* and `…Truncated Data (-600)` | +| 8 | (b) Execute | `phases.empty()` → error passthrough | `EbsdLib Error Passthrough - No Phase (-150)` | +| 9 | (b) Execute | All-slot Invalid-Phase default initialization | `Non-Contiguous Phase Index` (slot 1 keeps defaults) + `Class 1 Analytical Oracle` (slot 0) | +| 10 | (b) Execute | Per-phase fill: symmetry→structure index, trimmed name, lattice constants | `Class 1 Analytical Oracle` (2 phases, 2 Laue classes) | +| 11 | (b) Execute | `phaseID` out of ensemble range → `-19502` | *Not directly tested. Guard only trips if the file changes between preflight and execute; requires file-mutation injection.* | +| 12 | (c) Copy | Phase remap `< 1 → 1` then copy | `Class 1 Analytical Oracle` (points 2, 5) | +| 13 | (c) Copy | Euler interleave `3i/3i+1/3i+2` | `Class 1 Analytical Oracle` (18 values) | +| 14 | (c) Copy | Verbatim copies: IQ, CI, SEM Signal, Fit, X/Y Position | `Class 1 Analytical Oracle` (6 arrays element-wise) | +| 15 | (b)/(c) | Cancel checks (4 sites) | *Not directly tested. Requires cancel-signal injection; standard early-return pattern.* | + +## Test inventory + +| Test case | Status | Notes | +|-----------|--------|-------| +| `OrientationAnalysis::ReadAngDataFilter: Class 1 Analytical Oracle` | new-for-V&V | Class 1 + 4. Geometry (dims/spacing/origin/units), 8 cell arrays and 3 ensemble arrays asserted element-wise (≈70 assertions), all expected values hand-derived, float32-exact fixture. Inline data. | +| `OrientationAnalysis::ReadAngDataFilter: Non-Contiguous Phase Index` | new-for-V&V | Class 4. Sparse `# Phase 2`-only file → ensemble sized to 3 tuples, uncovered slot keeps Invalid-Phase defaults. Regression pin for the OOB fix (Deviation D3). | +| `OrientationAnalysis::ReadAngDataFilter: HexGrid Preflight Error (-19500)` | new-for-V&V | Value-add preflight rejection. | +| `OrientationAnalysis::ReadAngDataFilter: Missing GRID Preflight Error (-19501)` | new-for-V&V | Value-add preflight rejection. | +| `OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - No Phase (-150)` | new-for-V&V | Replaces the retired archive-based "Invalid Phase" test with an inline no-phase-header fixture; same error code asserted. | +| `OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - Truncated Data (-600)` | new-for-V&V | Replaces the retired archive-based "Invalid Columns & Rows" test with an inline truncated-data fixture; same error code asserted. | +| `OrientationAnalysis::ReadAngDataFilter: SIMPL Backwards Compatibility` | new-for-V&V | `DYNAMIC_SECTION` over the existing 6.4/6.5 conversion fixtures — the filter previously had **no** conversion test despite shipping the fixtures. | +| *(retired)* `OrientationAnalysis::ReadAngData: Exemplary Test` | retired | Exemplar comparison against `read_ang_test.dream3d` — **circular oracle** (exemplar generated from this filter's own output). Replaced by the Class 1 oracle above. | +| *(retired)* `OrientationAnalysis::ReadAngData: Invalid Phase` | retired | Archive-based `-150` test; superseded by the inline passthrough test. | +| *(retired)* `OrientationAnalysis::ReadAngData: Invalid Columns & Rows` | retired | Archive-based `-600` test; superseded by the inline passthrough test. | + +## Exemplar archive + +- **Archive:** None. **`read_ang_test.tar.gz` retired** this pass (SHA512 was `de7cd89d…e4b236`); its `download_test_data()` entry is removed from `test/CMakeLists.txt`. +- **Provenance:** `src/Plugins/OrientationAnalysis/vv/provenance/read_ang_test.md` — documents the circular-oracle status and the inline replacement. +- All current oracle data is inline in `test/ReadAngDataTest.cpp`; there is nothing to archive. + +## Deviations from DREAM3D 6.5.171 + +*Comparison run 2026-07-07 against the official DREAM3D 6.5.171 release on three byte-identical-input fixtures (toy `.ang`, Small IN100 `Slice_1.ang`, sparse-phase toy). All numeric outputs bit-identical on the supported-format fixtures. Full write-ups: `vv/deviations/ReadAngDataFilter.md`; working artifacts in `Code_Review/ReadAngDataFilter/`.* + +- `ReadAngDataFilter-D1` — MaterialName trailing space: legacy stores `"Nickel "`, SIMPLNX trims to `"Nickel"`. Demonstrated on both fixtures. Trust SIMPLNX. +- `ReadAngDataFilter-D2` — TEM/ACOM `.ang` variants get Nanometer units in legacy, Micrometer in SIMPLNX. Document-only: EDAX retired those files 10+ years ago. Either acceptable. +- `ReadAngDataFilter-D3` — **Legacy crash bug, empirically confirmed:** non-contiguous phase indices segfault 6.5.171 (OOB ensemble write, exit 139); SIMPLNX imports correctly (fixed this pass, test-pinned). Trust SIMPLNX. +- `ReadAngDataFilter-D4` — Error-code renumbering on rejection paths (`-1000`→`-19500` HexGrid, etc.). No data effect. Either acceptable. diff --git a/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md new file mode 100644 index 0000000000..4445a105e9 --- /dev/null +++ b/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md @@ -0,0 +1,80 @@ +# Deviations from DREAM3D 6.5.171: ReadAngDataFilter + +This file lists every documented behavioral difference between this SIMPLNX filter and its DREAM3D 6.5.171 equivalent. + +Entries are referenced by stable ID (`ReadAngDataFilter-D`) from the V&V report and from public migration guidance. The ID is stable across renames; the Filter UUID field is the permanent cross-reference anchor. + +Comparison run 2026-07-07 against the official DREAM3D 6.5.171 release on three fixtures (hand-authored 2-phase toy `.ang`, Small IN100 `Slice_1.ang`, and a non-contiguous-phase-index variant of the toy file). **All numeric outputs — cell arrays, ensemble arrays, and geometry — were bit-identical on the two supported-format fixtures.** The deviations below are the complete list of differences. + +--- + +## ReadAngDataFilter-D1 + +| Field | Value | +|---|---| +| **Deviation ID** | `ReadAngDataFilter-D1` | +| **Filter UUID** | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| **Status** | active | + +**Symptom:** Material names in the Ensemble Attribute Matrix carry a trailing space in 6.5.171 (`"Nickel "`) but are trimmed in SIMPLNX (`"Nickel"`). + +**Root cause:** Algorithmic choice. EbsdLib's `AngPhase::parseMaterialName()` (both legacy and 3.0.0) rejoins the header tokens with a trailing space. SIMPLNX `ReadAngData::loadMaterialInfo()` applies `StringUtilities::trimmed()` before storing the name; 6.5.171 stored the raw string. Demonstrated on both comparison fixtures (2026-07-07). + +**Affected users:** Any workflow that string-matches material names against `.dream3d` files produced by 6.5.171 (e.g., scripting against `MaterialName`). Visualization users will not notice. + +**Recommendation:** Trust SIMPLNX. The trailing space is a parsing artifact, not information; the trimmed name is the name as written in the `.ang` header. + +--- + +## ReadAngDataFilter-D2 + +| Field | Value | +|---|---| +| **Deviation ID** | `ReadAngDataFilter-D2` | +| **Filter UUID** | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| **Status** | active | + +**Symptom:** For `.ang` files whose header contains `# TEM data` or `# File Created from ACOM RES results`, 6.5.171 sets the Image Geometry length unit to **Nanometer**; SIMPLNX always sets **Micrometer**. + +**Root cause:** Algorithmic choice. Legacy `ReadAngData::readDataFile()` scanned the original header for the two TEM/ACOM marker strings and switched the unit; the SIMPLNX port hard-codes `LengthUnit::Micrometer` (`ReadAngDataFilter.cpp`, preflight geometry action). Deliberately not restored: EDAX retired those TEM/ACOM `.ang` variants more than a decade ago and no longer supports them, so no supported instrument produces such files. Source-level finding; not demonstrable with any supported fixture. + +**Affected users:** Only users importing archival TEM/ACOM `.ang` files from retired EDAX software. The unit is display/metadata only — the numeric spacing values are identical. + +**Recommendation:** Either acceptable. Users with archival TEM/ACOM files should manually set the geometry units to Nanometer (e.g., Set Image Geometry Units filter) after import. + +--- + +## ReadAngDataFilter-D3 + +| Field | Value | +|---|---| +| **Deviation ID** | `ReadAngDataFilter-D3` | +| **Filter UUID** | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| **Status** | active | +| **Bug flag** | **Legacy bug — crash, empirically confirmed** | + +**Symptom:** A `.ang` file whose phase sections do not start at index 1 (e.g., only a `# Phase 2` section) **crashes DREAM3D 6.5.171 with a segmentation fault** (PipelineRunner exit code 139, confirmed 2026-07-07). SIMPLNX imports the file correctly. + +**Root cause:** Bug in 6.5.171. Legacy `ReadAngData::loadMaterialInfo()` sizes the ensemble arrays to `phases.size() + 1` but writes each phase at `phase->getPhaseIndex()`; when indices are non-contiguous the write is out of bounds (a 2-tuple array written at index 2). SIMPLNX (fixed during this V&V pass) sizes the ensemble arrays to `maxPhaseIndex + 1` in preflight, initializes every slot to the "Invalid Phase" defaults (`CrystalStructures = 999`, `MaterialName = "Invalid Phase"`, zero lattice constants), overwrites the slots that have phase sections, and returns error `-19502` if a phase index falls outside the arrays. + +**Affected users:** Anyone importing a `.ang` file with non-contiguous phase indices into 6.5.171 (crash, potential silent memory corruption in earlier writes). Standard EDAX exports number phases contiguously from 1, so typical files are unaffected. + +**Recommendation:** Trust SIMPLNX. The 6.5.171 behavior is undefined (out-of-bounds write). Pinned by the `Non-Contiguous Phase Index` unit test in `test/ReadAngDataTest.cpp`. + +--- + +## ReadAngDataFilter-D4 + +| Field | Value | +|---|---| +| **Deviation ID** | `ReadAngDataFilter-D4` | +| **Filter UUID** | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| **Status** | active | + +**Symptom:** Error codes and messages for rejected files differ: HexGrid files fail with `-1000` in 6.5.171 vs `-19500` in SIMPLNX; a missing `GRID` header key fails with `-300` (from EbsdLib at execute) in 6.5.171 vs `-19501` at preflight in SIMPLNX; missing-file and wrong-extension errors (`-388` / `-997` in legacy `dataCheck()`) are handled by the `FileSystemPathParameter` in SIMPLNX with framework codes. + +**Root cause:** Algorithmic choice (framework error-handling differences). No data output is affected — these are rejection paths. + +**Affected users:** Only scripts that match on specific error codes/messages from pipeline logs. + +**Recommendation:** Either acceptable. Update any error-code matching to the SIMPLNX codes (`-19500` HexGrid, `-19501` missing GRID, `-19502` phase-index range). diff --git a/src/Plugins/OrientationAnalysis/vv/provenance/read_ang_test.md b/src/Plugins/OrientationAnalysis/vv/provenance/read_ang_test.md new file mode 100644 index 0000000000..d0360afe27 --- /dev/null +++ b/src/Plugins/OrientationAnalysis/vv/provenance/read_ang_test.md @@ -0,0 +1,40 @@ +# Exemplar Archive Provenance: read_ang_test.tar.gz — RETIRED + +This sidecar documents the **retirement** of `read_ang_test.tar.gz` during the ReadAngDataFilter V&V pass (2026-07-07). The archive is no longer referenced by any test or `download_test_data()` entry. + +--- + +## Archive identity (at retirement) + +| Field | Value | +|---|---| +| **Archive** | `read_ang_test.tar.gz` | +| **SHA512** | `de7cd89d925da01f291f44686964ec89d469659d0005219f9869afe26b8f62af278461ac3f5deb3afe7f3e65ec074ab3a1357d77a1a5f92eb3a1ea8cc5e4b236` | +| **Was used by tests** | `OrientationAnalysis::ReadAngData: Exemplary Test`, `…: Invalid Phase`, `…: Invalid Columns & Rows` (all retired/replaced) | +| **Retired by** | Michael Jackson | +| **Retired on** | 2026-07-07 | + +## Why it was retired + +**Circular oracle.** The archive bundled `read_ang_test.dream3d`, an exemplar generated by running this same filter and saving its output — so the "Exemplary Test" verified only that the filter still produced whatever it produced when the exemplar was made, not that the output was correct. Per the V&V policy (and the standing project rule that exemplar archives be retired in favor of inline toy data), it was replaced rather than regenerated. + +## Replacement + +The oracle is now **inline in `test/ReadAngDataTest.cpp`** (no archive at all): + +- A hand-authored toy `.ang` fixture (string literal, written to the binary test-output dir at runtime) with **Class 1 hand-derived expected values** for every output array and the geometry, plus **Class 4 invariants** (ensemble sizing, Invalid-Phase slot defaults). +- The archive's two invalid `.ang` error fixtures are replaced by inline variants (no-phase header → `-150`; truncated data → `-600`), plus new value-add error fixtures (HexGrid → `-19500`, missing GRID → `-19501`) and a non-contiguous-phase fixture. + +See the report's Oracle section: `src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md`. + +## Second-engineer oracle review + +Skipped — documented reason: the filter's value-add is deterministic data plumbing (grid math from header values, verbatim column copies, a documented TSL-symmetry enum mapping, string trim). Every expected value is mechanically derivable from the fixture text with no design freedom, so the author-bias failure mode the second-engineer review guards against has no room to operate. The oracle fixture and derivations are fully visible in one screen of test source for any reviewer to audit at sign-off. + +## Circular-oracle note + +This retirement **removes** a circular oracle. The replacement was derived by hand from the fixture text before any DREAM3D (legacy or NX) run — verified order: oracle first (2026-07-06), SIMPLNX reconciliation (zero discrepancies), then the 6.5.171 comparison (2026-07-07, bit-identical numerics). + +## Archive disposition + +The tar.gz remains in the GitHub [Data_Archive release](https://github.com/BlueQuartzSoftware/simplnx/releases/tag/Data_Archive) for historical checkouts of older commits; it must not be referenced by new code. From bc77bd924f23b6f993f3a3c2528246139219c644 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Tue, 7 Jul 2026 11:01:19 -0400 Subject: [PATCH 2/3] DOC: Set ReadAngDataFilter V&V report status to COMPLETE Signed-off-by: Michael Jackson --- .../OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp | 4 ++-- src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp | 6 +++--- src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp index 4e3f464d5a..a0673b92ea 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp @@ -92,8 +92,8 @@ Result<> ReadAngData::loadMaterialInfo(ebsdlib::AngReader* reader) const // This guard only trips if the file changed between preflight and execute. if(phaseID < 1 || static_cast(phaseID) >= numTuples) { - return MakeErrorResult(-19502, fmt::format("Phase index {} from the .ang file falls outside the Ensemble Attribute Matrix range [1, {}]. The input file may have changed since preflight.", - phaseID, numTuples - 1)); + return MakeErrorResult( + -19502, fmt::format("Phase index {} from the .ang file falls outside the Ensemble Attribute Matrix range [1, {}]. The input file may have changed since preflight.", phaseID, numTuples - 1)); } crystalStructures[phaseID] = phase->determineOrientationOpsIndex(); // EbsdLib's AngPhase::parseMaterialName() rejoins the name tokens with a trailing diff --git a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp index 9adbbbe99c..77f62c8551 100644 --- a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp +++ b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp @@ -199,11 +199,11 @@ TEST_CASE("OrientationAnalysis::ReadAngDataFilter: Class 1 Analytical Oracle", " REQUIRE_NOTHROW(dataStructure.getDataRefAs(k_DataContainerPath)); const auto& imageGeom = dataStructure.getDataRefAs(k_DataContainerPath); { - const SizeVec3 dims = imageGeom.getDimensions(); // NCOLS_EVEN=3, NROWS=2, one slice + const SizeVec3 dims = imageGeom.getDimensions(); // NCOLS_EVEN=3, NROWS=2, one slice REQUIRE(dims == SizeVec3(3, 2, 1)); - const FloatVec3 spacing = imageGeom.getSpacing(); // XSTEP=0.25, YSTEP=0.5, z hard-coded 1.0 + const FloatVec3 spacing = imageGeom.getSpacing(); // XSTEP=0.25, YSTEP=0.5, z hard-coded 1.0 REQUIRE(spacing == FloatVec3(0.25F, 0.5F, 1.0F)); - const FloatVec3 origin = imageGeom.getOrigin(); // hard-coded (0,0,0) + const FloatVec3 origin = imageGeom.getOrigin(); // hard-coded (0,0,0) REQUIRE(origin == FloatVec3(0.0F, 0.0F, 0.0F)); REQUIRE(imageGeom.getUnits() == IGeometry::LengthUnit::Micrometer); } diff --git a/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md index 083f5a72f9..f46b5fe614 100644 --- a/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md @@ -7,8 +7,8 @@ | SIMPLNX Human Name | Read EDAX EBSD Data (.ang) | | DREAM3D 6.5.171 equivalent | `ReadAngData` (SIMPL UUID `b8e128a8-c2a3-5e6c-a7ad-e4fb864e5d40`) — `Source/Plugins/OrientationAnalysis/OrientationAnalysisFilters/ReadAngData.{h,cpp}` | | Verified commit | ** | -| Status | READY FOR REVIEW — all V&V phases complete; verified-correct against independent oracle; legacy comparison bit-identical on numerics. Pending: sign-off. | -| Sign-off | *Michael Jackson — in progress, 2026-07-07* | +| Status | COMPLETE — all V&V phases complete; verified-correct against independent oracle; legacy comparison bit-identical on numerics. | +| Sign-off | Michael Jackson — 2026-07-07 | ## At a glance From 302e1db203960a0dfbe9fe63785cccf7288aa0a0 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Wed, 8 Jul 2026 15:41:33 -0400 Subject: [PATCH 3/3] BUG: Guard ReadAngData cell-count OOB read; review fixes Fixes from adversarial review of this PR: * Add a cell-count guard at the top of copyRawEbsdData (-19503). The Image Geometry is sized in preflight from the header's NCOLS_EVEN, but AngReader allocates its buffers from NCOLS_ODD; every column copy reads totalCells elements from those buffers, so a file changed between preflight and execute (or a malformed NCOLS_EVEN > NCOLS_ODD header) read past the reader's heap. copyRawEbsdData now returns Result<> and errors instead of reading out of bounds. This is the same TOCTOU class the ensemble-array guard already covered, now applied to the (much larger) cell copy path. * Split the loadMaterialInfo phase-range guard: phaseID < 1 (Phase 0 / negative) now returns -19502 with an accurate message ('.ang phase numbering starts at 1'), and phaseID >= ensemble count returns -19504 ('file may have changed since preflight'). Previously both shared one misleading 'file may have changed' message. * Add deviation D5 and a STATIC '# Phase 0' unit test asserting -19502. Legacy 6.5.171 accepted a Phase 0 section; SIMPLNX rejects it. This refutes the report's earlier claim that the phase-range guard needs file-mutation injection to reach. * Fix the test header's float32-exactness comment: the lattice constants (3.520, 2.950, 4.680) are not multiples of 1/8 and not exactly representable; they compare equal because strtof and the compiler round the same decimal literal identically. * Report honesty: mark the phases.empty() branch (row 8) as unreachable dead code (readFile returns -150 first); recount coverage to 13 of 17 enumerated paths. Signed-off-by: Michael Jackson --- .../Filters/Algorithms/ReadAngData.cpp | 43 ++++++++++++----- .../Filters/Algorithms/ReadAngData.hpp | 4 +- .../test/ReadAngDataTest.cpp | 46 ++++++++++++++++++- .../vv/ReadAngDataFilter.md | 17 ++++--- .../vv/deviations/ReadAngDataFilter.md | 20 +++++++- 5 files changed, 109 insertions(+), 21 deletions(-) diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp index a0673b92ea..7f5b6860c3 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp @@ -46,9 +46,7 @@ Result<> ReadAngData::operator()() } m_MessageHandler(IFilter::Message::Type::Info, "Copying cell data into the Image Geometry"); - copyRawEbsdData(&reader); - - return {}; + return copyRawEbsdData(&reader); } // ----------------------------------------------------------------------------- @@ -88,12 +86,20 @@ Result<> ReadAngData::loadMaterialInfo(ebsdlib::AngReader* reader) const for(const ebsdlib::AngPhase::Pointer& phase : phases) { const int32_t phaseID = phase->getPhaseIndex(); - // The ensemble arrays were sized in preflight from the same file's largest phase index. - // This guard only trips if the file changed between preflight and execute. - if(phaseID < 1 || static_cast(phaseID) >= numTuples) + // .ang phase numbering starts at 1. A phase index < 1 (Phase 0 or negative) is rejected: DREAM3D + // 6.5.171 tolerated a Phase 0 section (it skipped only negative indices and wrote Phase 0 into + // ensemble slot 0), so this is a documented behavior change vs legacy — see deviation + // ReadAngDataFilter-D5. A static "# Phase 0" fixture trips this deterministically. + if(phaseID < 1) + { + return MakeErrorResult(-19502, fmt::format("The .ang file declares phase index {}, but .ang phase numbering starts at 1 (Phase 0 and negative phases are not supported).", phaseID)); + } + // The ensemble arrays were sized in preflight from the same file's largest phase index, so an index + // at or above the array count can only mean the file changed between preflight and execute. + if(static_cast(phaseID) >= numTuples) { return MakeErrorResult( - -19502, fmt::format("Phase index {} from the .ang file falls outside the Ensemble Attribute Matrix range [1, {}]. The input file may have changed since preflight.", phaseID, numTuples - 1)); + -19504, fmt::format("Phase index {} from the .ang file is at or above the Ensemble Attribute Matrix count {}. The input file may have changed since preflight.", phaseID, numTuples)); } crystalStructures[phaseID] = phase->determineOrientationOpsIndex(); // EbsdLib's AngPhase::parseMaterialName() rejoins the name tokens with a trailing @@ -111,18 +117,31 @@ Result<> ReadAngData::loadMaterialInfo(ebsdlib::AngReader* reader) const } // ----------------------------------------------------------------------------- -void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const +Result<> ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const { const DataPath cellAttributeMatrixPath = m_InputValues->DataContainerName.createChildPath(m_InputValues->CellAttributeMatrixName); const auto& imageGeom = m_DataStructure.getDataRefAs(m_InputValues->DataContainerName); const size_t totalCells = imageGeom.getNumberOfCells(); + // The Image Geometry was sized in preflight from the file's column/row header (NumEvenCols x NumRows), + // but the reader allocates its data buffers from NumOddCols x NumRows. Every copy below reads + // totalCells elements out of those buffers, so if the reader actually read fewer elements (a file that + // changed between preflight and execute, or a malformed header where NCOLS_EVEN > NCOLS_ODD) the copies + // would read past the end of the reader's heap buffers. Guard against that out-of-bounds read. + if(reader->getNumberOfElements() < totalCells) + { + return MakeErrorResult(-19503, + fmt::format("The .ang reader produced {} scan points but the Image Geometry created at preflight expects {}. The input file may have changed since preflight, or its " + "column header (NCOLS_ODD/NCOLS_EVEN) is inconsistent.", + reader->getNumberOfElements(), totalCells)); + } + // Adjust the values of the 'phase' data to correct for invalid values and assign the read Phase Data into the actual DataArray { if(m_ShouldCancel) { - return; + return {}; } auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::AngFile::Phases)); int* phasePtr = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::PhaseData)); @@ -140,7 +159,7 @@ void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const { if(m_ShouldCancel) { - return; + return {}; } const auto* fComp0 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Phi1)); const auto* fComp1 = reinterpret_cast(reader->getPointerByName(ebsdlib::Ang::Phi)); @@ -157,7 +176,7 @@ void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const if(m_ShouldCancel) { - return; + return {}; } { @@ -195,4 +214,6 @@ void ReadAngData::copyRawEbsdData(ebsdlib::AngReader* reader) const auto& targetArray = m_DataStructure.getDataRefAs(cellAttributeMatrixPath.createChildPath(ebsdlib::Ang::YPosition)); std::copy(fComp0, fComp0 + totalCells, targetArray.begin()); } + + return {}; } diff --git a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp index 68a3bd031c..945c411302 100644 --- a/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp +++ b/src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.hpp @@ -57,8 +57,10 @@ class ORIENTATIONANALYSIS_EXPORT ReadAngData * arrays: remaps phase values < 1 to 1, interleaves phi1/PHI/phi2 into the 3-component * EulerAngles array, and copies the remaining columns verbatim. * @param reader The AngReader that has already successfully read the input file. + * @return Error result if the reader produced fewer scan points than the preflight-sized geometry + * expects (which would otherwise read past the reader's buffers). */ - void copyRawEbsdData(ebsdlib::AngReader* reader) const; + Result<> copyRawEbsdData(ebsdlib::AngReader* reader) const; }; } // namespace nx::core diff --git a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp index 77f62c8551..762e7ddbe2 100644 --- a/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp +++ b/src/Plugins/OrientationAnalysis/test/ReadAngDataTest.cpp @@ -18,8 +18,11 @@ * The toy .ang fixture below is hand-authored and every * expected value is hand-derived from the fixture text. * - * All fixture values are exactly representable in float32 (multiples of 1/8) - * so the expected values can be asserted with exact equality. + * The Euler/IQ/CI/position/fit column values are exactly representable in + * float32 (multiples of 1/8), so they are asserted with exact equality. The + * lattice constants (e.g. 3.520, 2.950, 4.680) are NOT exact float32 values; + * they compare equal because runtime strtof and the compiler's parse of the + * same decimal literal round identically to the same float32 bit pattern. * * The prior exemplar archive read_ang_test.tar.gz was a CIRCULAR ORACLE (the * exemplar .dream3d was generated from this filter's own output) and has been @@ -104,6 +107,21 @@ const std::string k_SparsePhaseBlock = R"(# Phase 2 # )"; +// A "# Phase 0" section. DREAM3D 6.5.171 tolerated this (it wrote Phase 0 into ensemble slot 0), +// but .ang phase numbering starts at 1, so SIMPLNX rejects it at execute with -19502. A static +// fixture trips this deterministically — no file-mutation injection is needed. +const std::string k_Phase0Block = R"(# Phase 0 +# MaterialName Nickel +# Formula Ni +# Info +# Symmetry 43 +# LatticeConstants 3.520 3.520 3.520 90.000 90.000 90.000 +# NumberFamilies 1 +# hklFamilies 1 1 1 1 8.469246 1 +# Categories 0 0 0 0 0 +# +)"; + // {} is the GRID line (or empty to omit it entirely) const std::string k_GridBlockFmt = R"({}# XSTEP: 0.250000 # YSTEP: 0.500000 @@ -362,6 +380,30 @@ TEST_CASE("OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - N REQUIRE(executeResult.result.errors()[0].code == -150); } +//------------------------------------------------------------------------------ +// Deviation D5: a "# Phase 0" section is accepted by legacy 6.5.171 but rejected +// by SIMPLNX at execute with -19502 (.ang phase numbering starts at 1). This is a +// STATIC fixture — it trips the guard deterministically, without any file-mutation +// injection between preflight and execute. +//------------------------------------------------------------------------------ +TEST_CASE("OrientationAnalysis::ReadAngDataFilter: Phase 0 rejected (-19502)", "[OrientationAnalysis][ReadAngDataFilter]") +{ + UnitTest::LoadPlugins(); + + const fs::path inputAngFile = WriteAngFile("read_ang_vv_phase0.ang", k_HeaderPrefix + k_Phase0Block + SqrGridBlock() + k_DataBlock); + + ReadAngDataFilter filter; + DataStructure dataStructure; + Arguments args = MakeDefaultArgs(inputAngFile); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); + + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(executeResult.result); + REQUIRE(executeResult.result.errors()[0].code == -19502); +} + //------------------------------------------------------------------------------ // EbsdLib error passthrough: the header declares 3x2 points but the data block // is truncated to 4 lines; AngReader reports -600 at execute. diff --git a/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md index f46b5fe614..f73824539b 100644 --- a/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/ReadAngDataFilter.md @@ -16,7 +16,7 @@ |------------------------|---------------| | Algorithm Relationship | **Minor changes.** Faithful port of legacy `ReadAngData` control flow with 4 deliberate deltas: material-name trim added (D1), TEM/ACOM Nanometer-units detection dropped (D2, obsolete file variants), non-contiguous phase-index handling fixed (D3, legacy crashes), and error-code renumbering (D4). The legacy PIMPL file-cache was dropped (no output effect). | | Oracle (confirmed) | **Confirmed.** **Class 1 (analytical) + Class 4 (invariant)**, scoped to the filter's value-add per the "don't re-test upstream" rule — EbsdLib (vcpkg 3.0.0) owns `.ang` parsing and is trusted (Class 2 boundary). A hand-authored inline toy `.ang` (3×2 grid, 2 phases, phase-0 points, all values float32-exact) with every expected value hand-derived from the fixture text. Encoded as 7 TEST_CASEs in `test/ReadAngDataTest.cpp`; all pass. SIMPLNX matched the oracle on every fixture with zero discrepancies. | -| Code paths enumerated | 13 of 15 paths exercised (see Code path coverage); the two gaps are cancel-signal paths and the file-changed-between-preflight-and-execute guard, both untestable without injection. | +| Code paths enumerated | 13 of 17 paths exercised (see Code path coverage); the gaps are one unreachable dead branch (`phases.empty()`), two file-changed/malformed-input guards (`-19503`, `-19504`), and the cancel-signal paths — all untestable without injection. | | Tests today | 7 test cases: Class 1+4 analytical oracle, non-contiguous phase-index invariant (regression pin for D3), 2 value-add preflight error paths (-19500/-19501), 2 EbsdLib error passthroughs (-150/-600), and SIMPL 6.4/6.5 backwards-compat (DYNAMIC_SECTION, new — the filter previously had no conversion test). All inline hand-built fixtures — no exemplar archive. | | Exemplar archive | **None — retired `read_ang_test.tar.gz`** (circular oracle: the exemplar `.dream3d` was generated from this filter's own output). `download_test_data()` entry removed from `test/CMakeLists.txt`; retirement documented in `vv/provenance/read_ang_test.md`. | | Legacy comparison | **Run (2026-07-07) vs the official DREAM3D 6.5.171 release.** Three fixtures: hand-authored toy, Small IN100 `Slice_1.ang` (189×201 production scan), non-contiguous-phase toy. On the two supported-format fixtures **all numeric outputs are bit-identical** (cell arrays, ensemble arrays, geometry). Differences: MaterialName trailing space (D1) and the non-contiguous-phase fixture, where **6.5.171 segfaults** (D3). | @@ -71,7 +71,8 @@ All 7 pass in the in-core `simplnx-rel` build. Reconciliation found zero SIMPLNX Line-by-line review performed via the `review-algorithm` skill after oracle reconciliation. All findings applied (all 7 tests still pass after rebuild): -- **Robustness (Critical):** fixed the latent out-of-bounds ensemble write on non-contiguous phase indices (preflight `maxPhaseIndex+1` sizing; full default initialization of every slot; `-19502` range guard in `loadMaterialInfo`). Legacy twin of this bug segfaults (Deviation D3). +- **Robustness (Critical):** fixed the latent out-of-bounds ensemble write on non-contiguous phase indices (preflight `maxPhaseIndex+1` sizing; full default initialization of every slot; `-19502`/`-19504` phase-range guards in `loadMaterialInfo`). Legacy twin of this bug segfaults (Deviation D3). +- **Robustness:** added a cell-count guard at the top of `copyRawEbsdData` (`-19503`). The geometry is sized in preflight from the header's `NCOLS_EVEN`, but the reader allocates its buffers from `NCOLS_ODD`; the copies read `totalCells` elements from those buffers, so a file that changed between preflight and execute (or a malformed `NCOLS_EVEN > NCOLS_ODD` header) would read past the reader's heap. The guard rejects that case instead of reading out of bounds. Found by the adversarial review. - **Dead code:** removed the vestigial legacy PIMPL (`ReadAngDataPrivate`, `Ang_Private_Data`, `m_AngDataPrivate` member, forward declaration), the never-firing `replace("MaterialName", "")`, unused `tDims`/`cDims` locals, the unused `FloatVec3Type` alias, and scaffold-generator boilerplate comments. - **API consistency:** `loadMaterialInfo` now returns `Result<>` instead of `std::pair`. - **Progress messaging:** added status messages before the EbsdLib read and the cell-data copy (the loops themselves are memcpy-speed; no throttled messenger warranted). @@ -79,7 +80,7 @@ Line-by-line review performed via the `review-algorithm` skill after oracle reco ## Code path coverage -*13 of 15 paths exercised. Source: `src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp` (198 lines) + preflight in `Filters/ReadAngDataFilter.cpp` (~265 lines).* Logical phases: **(a)** preflight (header-only read → output actions), **(b)** execute read + ensemble population (`loadMaterialInfo`), **(c)** cell-data copy (`copyRawEbsdData`). +*13 of 17 enumerated paths exercised; 1 is unreachable dead code (row 8) and 3 are file-changed/malformed-input guards or cancel checks that need injection (rows 11b, 15b, 16). Source: `src/Plugins/OrientationAnalysis/src/OrientationAnalysis/Filters/Algorithms/ReadAngData.cpp` + preflight in `Filters/ReadAngDataFilter.cpp`.* Logical phases: **(a)** preflight (header-only read → output actions), **(b)** execute read + ensemble population (`loadMaterialInfo`), **(c)** cell-data copy (`copyRawEbsdData`). | # | Phase | Path | Test case | |----|-------|------|-----------| @@ -90,14 +91,16 @@ Line-by-line review performed via the `review-algorithm` skill after oracle reco | 5 | (a) Preflight | Per-column array creation with int32/float32 type dispatch from `AngFields` | `Class 1 Analytical Oracle` (all 8 cell arrays exist with right types) | | 6 | (a) Preflight | Ensemble AM sized `maxPhaseIndex + 1`; CrystalStructures/LatticeConstants/MaterialName actions | `Class 1 Analytical Oracle` (3 tuples) + `Non-Contiguous Phase Index` (3 tuples from max index 2) | | 7 | (b) Execute | `readFile()` error → passthrough | `EbsdLib Error Passthrough - No Phase (-150)` *(−110/−100 variants are the same return statement)* and `…Truncated Data (-600)` | -| 8 | (b) Execute | `phases.empty()` → error passthrough | `EbsdLib Error Passthrough - No Phase (-150)` | +| 8 | (b) Execute | `phases.empty()` → error passthrough | *Unreachable dead code: `AngReader::readFile()` returns `-150` when the phase vector is empty, so `operator()` returns at the readFile error check (row 7) before `loadMaterialInfo` is ever entered with empty phases.* | | 9 | (b) Execute | All-slot Invalid-Phase default initialization | `Non-Contiguous Phase Index` (slot 1 keeps defaults) + `Class 1 Analytical Oracle` (slot 0) | | 10 | (b) Execute | Per-phase fill: symmetry→structure index, trimmed name, lattice constants | `Class 1 Analytical Oracle` (2 phases, 2 Laue classes) | -| 11 | (b) Execute | `phaseID` out of ensemble range → `-19502` | *Not directly tested. Guard only trips if the file changes between preflight and execute; requires file-mutation injection.* | +| 11a | (b) Execute | `phaseID < 1` (Phase 0 / negative) → `-19502` | `Phase 0 rejected (-19502)` — static `# Phase 0` fixture, trips deterministically (D5) | +| 11b | (b) Execute | `phaseID >= ensemble count` → `-19504` | *Not directly tested. Only reachable if the file changes between preflight and execute; requires file-mutation injection.* | | 12 | (c) Copy | Phase remap `< 1 → 1` then copy | `Class 1 Analytical Oracle` (points 2, 5) | | 13 | (c) Copy | Euler interleave `3i/3i+1/3i+2` | `Class 1 Analytical Oracle` (18 values) | | 14 | (c) Copy | Verbatim copies: IQ, CI, SEM Signal, Fit, X/Y Position | `Class 1 Analytical Oracle` (6 arrays element-wise) | -| 15 | (b)/(c) | Cancel checks (4 sites) | *Not directly tested. Requires cancel-signal injection; standard early-return pattern.* | +| 15b | (c) Copy | reader element count `< totalCells` → `-19503` (out-of-bounds read guard) | *Not directly tested. Only reachable if the file changes between preflight and execute or a malformed `NCOLS_EVEN > NCOLS_ODD` header; requires injection.* | +| 16 | (b)/(c) | Cancel checks (4 sites) | *Not directly tested. Requires cancel-signal injection; standard early-return pattern.* | ## Test inventory @@ -105,6 +108,7 @@ Line-by-line review performed via the `review-algorithm` skill after oracle reco |-----------|--------|-------| | `OrientationAnalysis::ReadAngDataFilter: Class 1 Analytical Oracle` | new-for-V&V | Class 1 + 4. Geometry (dims/spacing/origin/units), 8 cell arrays and 3 ensemble arrays asserted element-wise (≈70 assertions), all expected values hand-derived, float32-exact fixture. Inline data. | | `OrientationAnalysis::ReadAngDataFilter: Non-Contiguous Phase Index` | new-for-V&V | Class 4. Sparse `# Phase 2`-only file → ensemble sized to 3 tuples, uncovered slot keeps Invalid-Phase defaults. Regression pin for the OOB fix (Deviation D3). | +| `OrientationAnalysis::ReadAngDataFilter: Phase 0 rejected (-19502)` | new-for-V&V | Value-add execute rejection. Static `# Phase 0` fixture; asserts `-19502`. Pins deviation D5 (legacy accepted Phase 0) and refutes the earlier claim that the phase-range guard needs file-mutation injection to reach. | | `OrientationAnalysis::ReadAngDataFilter: HexGrid Preflight Error (-19500)` | new-for-V&V | Value-add preflight rejection. | | `OrientationAnalysis::ReadAngDataFilter: Missing GRID Preflight Error (-19501)` | new-for-V&V | Value-add preflight rejection. | | `OrientationAnalysis::ReadAngDataFilter: EbsdLib Error Passthrough - No Phase (-150)` | new-for-V&V | Replaces the retired archive-based "Invalid Phase" test with an inline no-phase-header fixture; same error code asserted. | @@ -128,3 +132,4 @@ Line-by-line review performed via the `review-algorithm` skill after oracle reco - `ReadAngDataFilter-D2` — TEM/ACOM `.ang` variants get Nanometer units in legacy, Micrometer in SIMPLNX. Document-only: EDAX retired those files 10+ years ago. Either acceptable. - `ReadAngDataFilter-D3` — **Legacy crash bug, empirically confirmed:** non-contiguous phase indices segfault 6.5.171 (OOB ensemble write, exit 139); SIMPLNX imports correctly (fixed this pass, test-pinned). Trust SIMPLNX. - `ReadAngDataFilter-D4` — Error-code renumbering on rejection paths (`-1000`→`-19500` HexGrid, etc.). No data effect. Either acceptable. +- `ReadAngDataFilter-D5` — A `# Phase 0` section is accepted by 6.5.171 but rejected by SIMPLNX at execute (`-19502`); `.ang` phase numbering starts at 1. Pinned by a static fixture. Trust SIMPLNX. diff --git a/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md b/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md index 4445a105e9..6a61c0ea7c 100644 --- a/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md +++ b/src/Plugins/OrientationAnalysis/vv/deviations/ReadAngDataFilter.md @@ -77,4 +77,22 @@ Comparison run 2026-07-07 against the official DREAM3D 6.5.171 release on three **Affected users:** Only scripts that match on specific error codes/messages from pipeline logs. -**Recommendation:** Either acceptable. Update any error-code matching to the SIMPLNX codes (`-19500` HexGrid, `-19501` missing GRID, `-19502` phase-index range). +**Recommendation:** Either acceptable. Update any error-code matching to the SIMPLNX codes (`-19500` HexGrid, `-19501` missing GRID, `-19502` phase index < 1, `-19503` reader/geometry element-count mismatch, `-19504` phase index above the ensemble count). + +--- + +## ReadAngDataFilter-D5 + +| Field | Value | +|---|---| +| **Deviation ID** | `ReadAngDataFilter-D5` | +| **Filter UUID** | `5b062816-79ac-47ce-93cb-e7966896bcbd` | +| **Status** | active | + +**Symptom:** A `.ang` file containing a `# Phase 0` section is accepted by DREAM3D 6.5.171 but rejected by SIMPLNX at execute with error `-19502`. + +**Root cause:** Algorithmic choice. Legacy `ReadAngData` skipped only *negative* phase indices; a Phase 0 section was written into ensemble slot 0 (the slot SIMPLNX reserves for the "Invalid Phase" defaults) and the import succeeded. TSL `.ang` phase numbering starts at 1, so SIMPLNX treats a phase index `< 1` as malformed and rejects it. (Legacy also had no dedicated code path for it — it happened to not crash.) This is enforced in `loadMaterialInfo` and pinned by the static `Phase 0 rejected (-19502)` unit test, which trips the guard deterministically without any file-mutation injection. + +**Affected users:** Only users with nonstandard `.ang` files that declare a Phase 0 section — rare, and such files carry no meaningful phase-0 crystallography in legacy output anyway (slot 0 collided with the invalid-phase slot). + +**Recommendation:** Trust SIMPLNX. Renumber the file's phases to start at 1. A Phase 0 section is outside the TSL `.ang` specification.