From ee3d0a66ff12f25ba6d61b7f5e0e8365a8c185ae Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Thu, 23 Jul 2026 21:38:21 -0400 Subject: [PATCH] VV/PERF: Create Element Array from Feature Array fully V&V'ed Summary: - Found and fixed 3 bugs (uncaught std::bad_cast in preflight when a NeighborList/StringArray was selected - parameter now constrained to ArrayType::DataArray; uncaught std::invalid_argument for a '/' in the created-array suffix - now clean preflight error -3021; end() iterator dereference (UB) on zero-tuple FeatureIds in the shared ValidateFeatureIdsToFeatureAttributeMatrixIndexing utility); - documented 3 deviations from DREAM3D 6.5.171 (D1 output-array naming for converted pipelines, D2 over-provisioned feature array accepted in NX vs legacy error -5555, D3 legacy bug: silent out-of-bounds garbage for negative feature ids vs NX error -5355) - numeric output bit-identical on valid input across float32/int32x3/bool fixtures; - retired 1 test (Parameter Check exercised parameter validation instead of the filter's empty-selection guard; replaced with a guard-specific test); - augmented existing tests with 6 inlined Class 1 (Analytical) + Class 4 (Invariant) test fixtures (21 ctest entries total, 13 of 14 code paths; cancel path excluded); - V&V deliverables (report, deviations) archived to OneDrive per program decision; no exemplar archive needed (all fixtures in-memory); - PERF: raw-pointer fast path for in-core DataStore in the copy kernel (removes ~3 virtual calls per component-element), FeatureIds min/max validation hoisted out of the per-selected-array loop, cancel check throttled; OOC and other store types keep the serial virtual path; - fixed Xmdf->Xdmf typo, documented error codes, memory note, and example pipelines in the user-facing doc. Signed-off-by: Michael Jackson --- .../CopyFeatureArrayToElementArrayFilter.md | 17 +- .../CopyFeatureArrayToElementArray.cpp | 82 +++- .../CopyFeatureArrayToElementArray.hpp | 10 +- .../CopyFeatureArrayToElementArrayFilter.cpp | 27 +- .../CopyFeatureArrayToElementArrayFilter.hpp | 4 +- .../CopyFeatureArrayToElementArrayTest.cpp | 421 +++++++++++++++++- .../CopyFeatureArrayToElementArrayFilter.md | 142 ++++++ .../gen_nx_pipeline.py | 49 -- .../gen_simpl_pipeline.py | 51 --- .../RotateSampleRefFrameFilter/make_input.py | 30 -- .../RotateSampleRefFrameFilter/results.md | 38 -- .../CopyFeatureArrayToElementArrayFilter.md | 65 +++ src/simplnx/Utilities/DataArrayUtilities.cpp | 7 +- 13 files changed, 725 insertions(+), 218 deletions(-) create mode 100644 src/Plugins/SimplnxCore/vv/CopyFeatureArrayToElementArrayFilter.md delete mode 100644 src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_nx_pipeline.py delete mode 100644 src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_simpl_pipeline.py delete mode 100644 src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/make_input.py delete mode 100644 src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/results.md create mode 100644 src/Plugins/SimplnxCore/vv/deviations/CopyFeatureArrayToElementArrayFilter.md diff --git a/src/Plugins/SimplnxCore/docs/CopyFeatureArrayToElementArrayFilter.md b/src/Plugins/SimplnxCore/docs/CopyFeatureArrayToElementArrayFilter.md index fa5affa7dd..96b0cc3b85 100644 --- a/src/Plugins/SimplnxCore/docs/CopyFeatureArrayToElementArrayFilter.md +++ b/src/Plugins/SimplnxCore/docs/CopyFeatureArrayToElementArrayFilter.md @@ -6,12 +6,27 @@ Core (Memory/Management) ## Description -This **Filter** copies the values associated with a **Feature** to all the **Elements** that belong to that **Feature**. Xmdf visualization files write only the **Element** attributes, so if the user wants to display a spatial map of a **Feature** level attribute, this **Filter** will transfer that information down to the **Element** level. +This **Filter** copies the values associated with a **Feature** to all the **Elements** that belong to that **Feature**. For every **Element** `i`, the created array's tuple is the selected **Feature** array's tuple at index `FeatureIds[i]`. Xdmf visualization files write only the **Element** attributes, so if the user wants to display a spatial map of a **Feature** level attribute, this **Filter** will transfer that information down to the **Element** level. + +Multiple **Feature** arrays may be selected and copied in a single filter instance; all selected arrays must have the same number of tuples. Each created **Element** array is placed next to the selected *Cell Feature Ids* array and is named by appending the *Created Array Suffix* to the source array's name (e.g., `AvgTemp` with suffix `_Cell` creates `AvgTemp_Cell`). The created arrays keep the source array's data type and component layout. + +Note that each created array is **Element**-sized (number of **Elements** × components × bytes per value), so selecting many **Feature** arrays multiplies the additional memory required. + +### Input Validation + +- All selected **Feature** arrays must have the same number of tuples (error -3020). +- The *Created Array Suffix* must not contain a `/` character (error -3021). +- Every value in the *Cell Feature Ids* array must be non-negative. A negative value stops the filter with an error (-5355). +- The largest **Feature** Id must be a valid tuple index into each selected **Feature** array; otherwise the filter stops with an error (-5351). +- A **Feature** array with *more* tuples than the largest **Feature** Id requires is accepted — the extra tuples are simply never read. (Legacy DREAM3D 6.5 rejected this case.) % Auto generated parameter table will be inserted here ## Example Pipelines +- (02) Image Segmentation (ITKImageProcessing) +- (04) Porosity Analysis (ITKImageProcessing) + ## License & Copyright Please see the description file distributed with this **Plugin** diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.cpp index b4ef567514..e49bfad2e5 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.cpp @@ -1,10 +1,13 @@ #include "CopyFeatureArrayToElementArray.hpp" #include "simplnx/DataStructure/DataArray.hpp" +#include "simplnx/DataStructure/DataStore.hpp" #include "simplnx/Utilities/DataArrayUtilities.hpp" #include "simplnx/Utilities/ParallelAlgorithmUtilities.hpp" #include "simplnx/Utilities/ParallelDataAlgorithm.hpp" +#include + using namespace nx::core; namespace @@ -15,17 +18,45 @@ class CopyFeatureArrayToElementArrayImpl public: using StoreType = AbstractDataStore; - CopyFeatureArrayToElementArrayImpl(const IDataArray* selectedFeatureArray, const Int32AbstractDataStore& featureIdsStore, IDataArray* createdArray, const std::atomic_bool& shouldCancel) - : m_SelectedFeature(selectedFeatureArray->template getIDataStoreRefAs()) + CopyFeatureArrayToElementArrayImpl(const IDataArray& selectedFeatureArray, const Int32AbstractDataStore& featureIdsStore, IDataArray& createdArray, const std::atomic_bool& shouldCancel) + : m_SelectedFeatureStore(selectedFeatureArray.getIDataStoreRefAs()) , m_FeatureIdsStore(featureIdsStore) - , m_CreatedStore(createdArray->template getIDataStoreRefAs()) + , m_CreatedStore(createdArray.getIDataStoreRefAs()) , m_ShouldCancel(shouldCancel) { + // Raw-pointer fast path is only taken when all three stores are concrete in-memory + // DataStore objects. Each thread then writes a disjoint index range of a plain + // buffer, so there is no shared mutable store state. Any other store type (e.g. + // out-of-core) falls back to the virtual AbstractDataStore access path, which runs + // serially because IParallelAlgorithm disables parallelization for OOC data. + const auto* selectedFeatureStorePtr = dynamic_cast*>(&m_SelectedFeatureStore); + const auto* featureIdsStorePtr = dynamic_cast*>(&m_FeatureIdsStore); + auto* createdStorePtr = dynamic_cast*>(&m_CreatedStore); + if(selectedFeatureStorePtr != nullptr && featureIdsStorePtr != nullptr && createdStorePtr != nullptr) + { + m_SourcePtr = selectedFeatureStorePtr->data(); + m_FeatureIdsPtr = featureIdsStorePtr->data(); + m_DestPtr = createdStorePtr->data(); + } } void operator()(const Range& range) const { - const usize totalFeatureArrayComponents = m_SelectedFeature.getNumberOfComponents(); + const usize numComps = m_SelectedFeatureStore.getNumberOfComponents(); + + if(m_SourcePtr != nullptr) + { + for(usize i = range.min(); i < range.max(); ++i) + { + if((i & 0xFFFFULL) == 0ULL && m_ShouldCancel) + { + return; + } + // Copy the feature tuple indexed by this cell's feature id down to the cell-level array + std::copy_n(m_SourcePtr + numComps * static_cast(m_FeatureIdsPtr[i]), numComps, m_DestPtr + numComps * i); + } + return; + } for(usize i = range.min(); i < range.max(); ++i) { @@ -34,25 +65,28 @@ class CopyFeatureArrayToElementArrayImpl return; } - for(usize faComp = 0; faComp < totalFeatureArrayComponents; faComp++) + const auto featureId = static_cast(m_FeatureIdsStore[i]); + for(usize comp = 0; comp < numComps; comp++) { - // Get the feature identifier (or what ever the user has selected as their "Feature" identifier - m_CreatedStore[totalFeatureArrayComponents * i + faComp] = m_SelectedFeature[totalFeatureArrayComponents * m_FeatureIdsStore[i] + faComp]; + m_CreatedStore[numComps * i + comp] = m_SelectedFeatureStore[numComps * featureId + comp]; } } } private: - const StoreType& m_SelectedFeature; + const StoreType& m_SelectedFeatureStore; const Int32AbstractDataStore& m_FeatureIdsStore; StoreType& m_CreatedStore; const std::atomic_bool& m_ShouldCancel; + const T* m_SourcePtr = nullptr; + const int32* m_FeatureIdsPtr = nullptr; + T* m_DestPtr = nullptr; }; } // namespace // ----------------------------------------------------------------------------- CopyFeatureArrayToElementArray::CopyFeatureArrayToElementArray(DataStructure& dataStructure, const IFilter::MessageHandler& mesgHandler, const std::atomic_bool& shouldCancel, - CopyFeatureArrayToElementArrayInputValues* inputValues) + const CopyFeatureArrayToElementArrayInputValues* inputValues) : m_DataStructure(dataStructure) , m_InputValues(inputValues) , m_ShouldCancel(shouldCancel) @@ -66,24 +100,38 @@ CopyFeatureArrayToElementArray::~CopyFeatureArrayToElementArray() noexcept = def // ----------------------------------------------------------------------------- Result<> CopyFeatureArrayToElementArray::operator()() { + if(m_InputValues->SelectedFeatureArrayPaths.empty()) + { + return {}; + } + const auto& featureIds = m_DataStructure.getDataRefAs(m_InputValues->FeatureIdsPath); - for(const auto& selectedFeatureArrayPath : m_InputValues->SelectedFeatureArrayPaths) + // Validate the FeatureIds value range once, against the first selected array. Preflight has + // already verified (error -3020) that every selected feature array has the same tuple count, + // so a single validation covers all of them and avoids re-scanning FeatureIds per array. + auto validateNumFeatResult = ValidateFeatureIdsToFeatureAttributeMatrixIndexing(m_DataStructure, m_InputValues->SelectedFeatureArrayPaths[0], featureIds, false, m_MessageHandler); + if(validateNumFeatResult.invalid()) { - DataPath createdArrayPath = m_InputValues->FeatureIdsPath.replaceName(selectedFeatureArrayPath.getTargetName() + m_InputValues->CreatedArraySuffix); - const auto* selectedFeatureArray = m_DataStructure.getDataAs(selectedFeatureArrayPath); + return validateNumFeatResult; + } - auto validateNumFeatResult = ValidateFeatureIdsToFeatureAttributeMatrixIndexing(m_DataStructure, selectedFeatureArrayPath, featureIds, false, m_MessageHandler); - if(validateNumFeatResult.invalid()) + for(const auto& selectedFeatureArrayPath : m_InputValues->SelectedFeatureArrayPaths) + { + if(m_ShouldCancel) { - return validateNumFeatResult; + return {}; } + DataPath createdArrayPath = m_InputValues->FeatureIdsPath.replaceName(selectedFeatureArrayPath.getTargetName() + m_InputValues->CreatedArraySuffix); + const auto& selectedFeatureArray = m_DataStructure.getDataRefAs(selectedFeatureArrayPath); + auto& createdArray = m_DataStructure.getDataRefAs(createdArrayPath); + m_MessageHandler(IFilter::ProgressMessage{IFilter::ProgressMessage::Type::Info, fmt::format("Copying data into target array '{}'...", createdArrayPath.toString())}); ParallelDataAlgorithm dataAlg; dataAlg.setRange(0, featureIds.getNumberOfTuples()); - ExecuteParallelFunction<::CopyFeatureArrayToElementArrayImpl>(selectedFeatureArray->getDataType(), dataAlg, selectedFeatureArray, featureIds.getDataStoreRef(), - m_DataStructure.getDataAs(createdArrayPath), m_ShouldCancel); + dataAlg.requireArraysInMemory({&featureIds, &selectedFeatureArray, &createdArray}); + ExecuteParallelFunction<::CopyFeatureArrayToElementArrayImpl>(selectedFeatureArray.getDataType(), dataAlg, selectedFeatureArray, featureIds.getDataStoreRef(), createdArray, m_ShouldCancel); } return {}; diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.hpp index 5738eb3a1c..5c414f0b9c 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.hpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.hpp @@ -21,14 +21,14 @@ struct SIMPLNXCORE_EXPORT CopyFeatureArrayToElementArrayInputValues /** * @class CopyFeatureArrayToElementArray - * @brief This algorithm implements support code for the CopyFeatureArrayToElementArrayFilter + * @brief Copies each selected Feature-level array down to the Element (cell) level: for every + * Element i, the created array's tuple is the source array's tuple at index FeatureIds[i]. */ - class SIMPLNXCORE_EXPORT CopyFeatureArrayToElementArray { public: CopyFeatureArrayToElementArray(DataStructure& dataStructure, const IFilter::MessageHandler& mesgHandler, const std::atomic_bool& shouldCancel, - CopyFeatureArrayToElementArrayInputValues* inputValues); + const CopyFeatureArrayToElementArrayInputValues* inputValues); ~CopyFeatureArrayToElementArray() noexcept; CopyFeatureArrayToElementArray(const CopyFeatureArrayToElementArray&) = delete; @@ -36,6 +36,10 @@ class SIMPLNXCORE_EXPORT CopyFeatureArrayToElementArray CopyFeatureArrayToElementArray& operator=(const CopyFeatureArrayToElementArray&) = delete; CopyFeatureArrayToElementArray& operator=(CopyFeatureArrayToElementArray&&) noexcept = delete; + /** + * @brief Runs the copy for every selected feature array. + * @return Invalid Result on FeatureIds range-validation failure (-5355 / -5351). + */ Result<> operator()(); private: diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.cpp index 2662e08f45..6a6d7686ad 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.cpp @@ -6,6 +6,8 @@ #include "simplnx/DataStructure/DataPath.hpp" #include "simplnx/Filter/Actions/CreateArrayAction.hpp" #include "simplnx/Parameters/ArraySelectionParameter.hpp" +// DataObjectNameParameter.hpp and MultiPathSelectionParameter.hpp provide the SIMPLConversion +// converters (LinkedPathCreation..., SingleToMultiDataPathSelection...) used in FromSIMPLJson() #include "simplnx/Parameters/DataObjectNameParameter.hpp" #include "simplnx/Parameters/MultiArraySelectionParameter.hpp" #include "simplnx/Parameters/MultiPathSelectionParameter.hpp" @@ -52,9 +54,9 @@ Parameters CopyFeatureArrayToElementArrayFilter::parameters() const // Create the parameter descriptors that are needed for this filter params.insertSeparator(Parameters::Separator{"Input Feature Data"}); - params.insert(std::make_unique(k_SelectedFeatureArrayPath_Key, "Feature Data to Copy to Cell Data", + params.insert(std::make_unique(k_SelectedFeatureArrayPaths_Key, "Feature Data to Copy to Cell Data", "The DataPath to the feature data that should be copied to the cell level", MultiArraySelectionParameter::ValueType{}, - MultiArraySelectionParameter::AllowedTypes{IArray::ArrayType::Any}, nx::core::GetAllDataTypes())); + MultiArraySelectionParameter::AllowedTypes{IArray::ArrayType::DataArray}, nx::core::GetAllDataTypes())); params.insertSeparator(Parameters::Separator{"Input Cell Data"}); params.insert(std::make_unique(k_CellFeatureIdsArrayPath_Key, "Cell Feature Ids", "Specifies to which feature each cell belongs.", DataPath({"Cell Data", "FeatureIds"}), @@ -81,9 +83,9 @@ IFilter::UniquePointer CopyFeatureArrayToElementArrayFilter::clone() const IFilter::PreflightResult CopyFeatureArrayToElementArrayFilter::preflightImpl(const DataStructure& dataStructure, const Arguments& filterArgs, const MessageHandler& messageHandler, const std::atomic_bool& shouldCancel, const ExecutionContext& executionContext) const { - const auto pSelectedFeatureArrayPathsValue = filterArgs.value(k_SelectedFeatureArrayPath_Key); - const auto pFeatureIdsArrayPathValue = filterArgs.value(k_CellFeatureIdsArrayPath_Key); - const auto createdArraySuffix = filterArgs.value(k_CreatedArraySuffix_Key); + const auto pSelectedFeatureArrayPathsValue = filterArgs.value(k_SelectedFeatureArrayPaths_Key); + const auto pFeatureIdsArrayPathValue = filterArgs.value(k_CellFeatureIdsArrayPath_Key); + const auto pCreatedArraySuffixValue = filterArgs.value(k_CreatedArraySuffix_Key); nx::core::Result resultOutputActions; std::vector preflightUpdatedValues; @@ -93,6 +95,11 @@ IFilter::PreflightResult CopyFeatureArrayToElementArrayFilter::preflightImpl(con return MakePreflightErrorResult(nx::core::FilterParameter::Constants::k_Validate_Empty_Value, "You must select at least one feature data array to copy to an element data array."); } + if(pCreatedArraySuffixValue.find('/') != std::string::npos) + { + return MakePreflightErrorResult(-3021, fmt::format("The Created Array Suffix '{}' must not contain a '/' character.", pCreatedArraySuffixValue)); + } + auto tupleValidityCheck = dataStructure.validateNumberOfTuples(pSelectedFeatureArrayPathsValue); if(!tupleValidityCheck) { @@ -101,14 +108,14 @@ IFilter::PreflightResult CopyFeatureArrayToElementArrayFilter::preflightImpl(con const auto& featureIdsArray = dataStructure.getDataRefAs(pFeatureIdsArrayPathValue); const IDataStore& featureIdsArrayStore = featureIdsArray.getIDataStoreRef(); - const std::vector& tDims = featureIdsArrayStore.getTupleShape(); + const std::vector& tupleShape = featureIdsArrayStore.getTupleShape(); for(const auto& selectedFeatureArrayPath : pSelectedFeatureArrayPathsValue) { - DataPath createdArrayPath = pFeatureIdsArrayPathValue.replaceName(selectedFeatureArrayPath.getTargetName() + createdArraySuffix); + DataPath createdArrayPath = pFeatureIdsArrayPathValue.replaceName(selectedFeatureArrayPath.getTargetName() + pCreatedArraySuffixValue); const auto& selectedFeatureArray = dataStructure.getDataRefAs(selectedFeatureArrayPath); DataType dataType = selectedFeatureArray.getDataType(); - auto createArrayAction = std::make_unique(dataType, tDims, selectedFeatureArray.getComponentShape(), createdArrayPath); + auto createArrayAction = std::make_unique(dataType, tupleShape, selectedFeatureArray.getComponentShape(), createdArrayPath); resultOutputActions.value().appendAction(std::move(createArrayAction)); } @@ -120,7 +127,7 @@ Result<> CopyFeatureArrayToElementArrayFilter::executeImpl(DataStructure& dataSt const std::atomic_bool& shouldCancel, const ExecutionContext& executionContext) const { CopyFeatureArrayToElementArrayInputValues inputValues; - inputValues.SelectedFeatureArrayPaths = filterArgs.value(k_SelectedFeatureArrayPath_Key); + inputValues.SelectedFeatureArrayPaths = filterArgs.value(k_SelectedFeatureArrayPaths_Key); inputValues.FeatureIdsPath = filterArgs.value(k_CellFeatureIdsArrayPath_Key); inputValues.CreatedArraySuffix = filterArgs.value(k_CreatedArraySuffix_Key); @@ -144,7 +151,7 @@ Result CopyFeatureArrayToElementArrayFilter::FromSIMPLJson(const nloh std::vector> results; results.push_back( - SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedFeatureArrayPathKey, k_SelectedFeatureArrayPath_Key)); + SIMPLConversion::ConvertParameter(args, json, SIMPL::k_SelectedFeatureArrayPathKey, k_SelectedFeatureArrayPaths_Key)); results.push_back(SIMPLConversion::ConvertParameter(args, json, SIMPL::k_FeatureIdsArrayPathKey, k_CellFeatureIdsArrayPath_Key)); // Do NOT map the legacy CreatedArrayName onto k_CreatedArraySuffix_Key: the legacy filter converted a // single array whose output name was CreatedArrayName, but in SIMPLNX that string would be appended to diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.hpp index fe441a3da6..c0c8ed05c1 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.hpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyFeatureArrayToElementArrayFilter.hpp @@ -10,7 +10,7 @@ namespace nx::core /** * @class CopyFeatureArrayToElementArrayFilter * @brief This Filter copies the values associated with a Feature to all the Elements - * that belong to that Feature. Xmdf visualization files write only the Element attributes, + * that belong to that Feature. Xdmf visualization files write only the Element attributes, * so if the user wants to display a spatial map of a Feature level attribute, * this Filter will transfer that information down to the Element level. */ @@ -27,7 +27,7 @@ class SIMPLNXCORE_EXPORT CopyFeatureArrayToElementArrayFilter : public IFilter CopyFeatureArrayToElementArrayFilter& operator=(CopyFeatureArrayToElementArrayFilter&&) noexcept = delete; // Parameter Keys - static constexpr StringLiteral k_SelectedFeatureArrayPath_Key = "selected_feature_array_paths"; + static constexpr StringLiteral k_SelectedFeatureArrayPaths_Key = "selected_feature_array_paths"; static constexpr StringLiteral k_CellFeatureIdsArrayPath_Key = "feature_ids_path"; static constexpr StringLiteral k_CreatedArraySuffix_Key = "created_array_suffix"; diff --git a/src/Plugins/SimplnxCore/test/CopyFeatureArrayToElementArrayTest.cpp b/src/Plugins/SimplnxCore/test/CopyFeatureArrayToElementArrayTest.cpp index 1bf19975e3..4731814fa0 100644 --- a/src/Plugins/SimplnxCore/test/CopyFeatureArrayToElementArrayTest.cpp +++ b/src/Plugins/SimplnxCore/test/CopyFeatureArrayToElementArrayTest.cpp @@ -2,8 +2,11 @@ #include "SimplnxCore/SimplnxCore_test_dirs.hpp" #include "simplnx/Core/Application.hpp" +#include "simplnx/DataStructure/AttributeMatrix.hpp" #include "simplnx/DataStructure/DataArray.hpp" #include "simplnx/DataStructure/DataStore.hpp" +#include "simplnx/DataStructure/Geometry/ImageGeom.hpp" +#include "simplnx/DataStructure/NeighborList.hpp" #include "simplnx/Parameters/StringParameter.hpp" #include "simplnx/Pipeline/Pipeline.hpp" #include "simplnx/Pipeline/PipelineFilter.hpp" @@ -11,7 +14,6 @@ #include #include -#include using namespace nx::core; namespace fs = std::filesystem; @@ -24,20 +26,135 @@ const std::string k_FeatureDataArrayName("Feature Data Array"); const std::string k_CellTempArraySuffix("_ToCell"); const DataPath k_CellTempArrayPath({k_FeatureTemperatureName + k_CellTempArraySuffix}); const DataPath k_CellFeatureArrayPath({k_FeatureDataArrayName + k_CellTempArraySuffix}); + +// Class 1 (Analytical) fixture: hand-built input whose expected output is derived by hand. +// See src/Plugins/SimplnxCore/vv/CopyFeatureArrayToElementArrayFilter.md (Oracle section). +namespace AnalyticalFixtures +{ +const std::string k_ImageGeometryName("Image Geometry"); +const std::string k_CellDataName("Cell Data"); +const std::string k_CellFeatureDataName("Cell Feature Data"); +const std::string k_AvgTempName("AvgTemp"); +const std::string k_RGBName("RGB"); +const std::string k_ActiveName("Active"); +const std::string k_Suffix("_Cell"); + +constexpr usize k_RGBComponentCount = 3; + +const DataPath k_FeatureIdsPath({k_ImageGeometryName, k_CellDataName, k_CellFeatureIdsArrayName}); +const DataPath k_FeatureAMPath({k_ImageGeometryName, k_CellFeatureDataName}); +const DataPath k_AvgTempPath({k_ImageGeometryName, k_CellFeatureDataName, k_AvgTempName}); +const DataPath k_RGBPath({k_ImageGeometryName, k_CellFeatureDataName, k_RGBName}); +const DataPath k_ActivePath({k_ImageGeometryName, k_CellFeatureDataName, k_ActiveName}); +const DataPath k_AvgTempCellPath({k_ImageGeometryName, k_CellDataName, k_AvgTempName + k_Suffix}); +const DataPath k_RGBCellPath({k_ImageGeometryName, k_CellDataName, k_RGBName + k_Suffix}); +const DataPath k_ActiveCellPath({k_ImageGeometryName, k_CellDataName, k_ActiveName + k_Suffix}); + +// 4 x 3 x 1 (X,Y,Z) image geometry: 12 cells, 4 features (ids 0-3). +const std::vector k_FeatureIds = {0, 1, 1, 2, 2, 0, 3, 1, 3, 3, 0, 2}; + +// Feature-level source values (4 tuples). +const std::vector k_AvgTemp = {10.5F, 20.25F, -30.75F, 40.125F}; +const std::vector k_RGB = {1, 2, 3, 40, 50, 60, -7, 8, -9, 100, 200, 127}; +const std::vector k_Active = {false, true, true, false}; + +// Hand-derived expected outputs: out[i*C + c] = source[FeatureIds[i]*C + c]. +// AvgTemp_Cell[i] = AvgTemp[FeatureIds[i]]: +// fid = [0, 1, 1, 2, 2, 0, 3, 1, 3, 3, 0, 2 ] +const std::vector k_ExpectedAvgTempCell = {10.5F, 20.25F, 20.25F, -30.75F, -30.75F, 10.5F, 40.125F, 20.25F, 40.125F, 40.125F, 10.5F, -30.75F}; + +// RGB_Cell tuple i = RGB tuple FeatureIds[i] (3 components per tuple): +const std::vector k_ExpectedRGBCell = { + 1, 2, 3, // cell 0, fid 0 + 40, 50, 60, // cell 1, fid 1 + 40, 50, 60, // cell 2, fid 1 + -7, 8, -9, // cell 3, fid 2 + -7, 8, -9, // cell 4, fid 2 + 1, 2, 3, // cell 5, fid 0 + 100, 200, 127, // cell 6, fid 3 + 40, 50, 60, // cell 7, fid 1 + 100, 200, 127, // cell 8, fid 3 + 100, 200, 127, // cell 9, fid 3 + 1, 2, 3, // cell 10, fid 0 + -7, 8, -9, // cell 11, fid 2 +}; + +// Active_Cell[i] = Active[FeatureIds[i]]: +const std::vector k_ExpectedActiveCell = {false, true, true, true, true, false, false, true, false, false, false, true}; + +// Builds the 4x3x1 ImageGeom fixture with a Cell AttributeMatrix (FeatureIds) and a +// Feature AttributeMatrix (AvgTemp float32/1-comp, RGB int32/3-comp, Active bool/1-comp). +DataStructure CreateFixture() +{ + DataStructure dataStructure; + + auto* imageGeomPtr = ImageGeom::Create(dataStructure, k_ImageGeometryName); + imageGeomPtr->setDimensions(SizeVec3{4, 3, 1}); // X, Y, Z + imageGeomPtr->setSpacing(FloatVec3{1.0F, 1.0F, 1.0F}); + imageGeomPtr->setOrigin(FloatVec3{0.0F, 0.0F, 0.0F}); + + // AttributeMatrix tuple shape is slowest-to-fastest (Z, Y, X) + auto* cellAMPtr = AttributeMatrix::Create(dataStructure, k_CellDataName, std::vector{1, 3, 4}, imageGeomPtr->getId()); + imageGeomPtr->setCellData(*cellAMPtr); + + auto* featureIdsPtr = Int32Array::CreateWithStore>(dataStructure, k_CellFeatureIdsArrayName, {1, 3, 4}, {1}, cellAMPtr->getId()); + auto& featureIdsStoreRef = featureIdsPtr->getDataStoreRef(); + for(usize i = 0; i < k_FeatureIds.size(); i++) + { + featureIdsStoreRef[i] = k_FeatureIds[i]; + } + + auto* featureAMPtr = AttributeMatrix::Create(dataStructure, k_CellFeatureDataName, std::vector{4}, imageGeomPtr->getId()); + + auto* avgTempPtr = Float32Array::CreateWithStore>(dataStructure, k_AvgTempName, {4}, {1}, featureAMPtr->getId()); + auto& avgTempStoreRef = avgTempPtr->getDataStoreRef(); + for(usize i = 0; i < k_AvgTemp.size(); i++) + { + avgTempStoreRef[i] = k_AvgTemp[i]; + } + + auto* rgbPtr = Int32Array::CreateWithStore>(dataStructure, k_RGBName, {4}, {k_RGBComponentCount}, featureAMPtr->getId()); + auto& rgbStoreRef = rgbPtr->getDataStoreRef(); + for(usize i = 0; i < k_RGB.size(); i++) + { + rgbStoreRef[i] = k_RGB[i]; + } + + auto* activePtr = BoolArray::CreateWithStore>(dataStructure, k_ActiveName, {4}, {1}, featureAMPtr->getId()); + auto& activeStoreRef = activePtr->getDataStoreRef(); + for(usize i = 0; i < k_Active.size(); i++) + { + activeStoreRef[i] = k_Active[i]; + } + + return dataStructure; +} + +Arguments CreateArguments() +{ + Arguments args; + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{k_AvgTempPath, k_RGBPath, k_ActivePath})); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(k_FeatureIdsPath)); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any(k_Suffix)); + return args; +} +} // namespace AnalyticalFixtures } // namespace -TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Parameter Check", "[Core][CopyFeatureArrayToElementArrayFilter]") +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - Empty selection (filter guard)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") { UnitTest::LoadPlugins(); - // Instantiate the filter, a DataStructure object and an Arguments Object CopyFeatureArrayToElementArrayFilter filter; DataStructure dataStructure; Arguments args; - // Create default Parameters for the filter. - args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPath_Key, std::make_any>(std::vector{})); - args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath{})); + // The FeatureIds path must be VALID so that parameter validation passes and the + // filter's own empty-selection guard in preflightImpl()/executeImpl() is what fires. + Int32Array::CreateWithStore>(dataStructure, k_CellFeatureIdsArrayName, {30}, {1}); + + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{})); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath({k_CellFeatureIdsArrayName}))); args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any("")); // Preflight the filter and check result @@ -61,7 +178,7 @@ TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Parameter Check", UnitTest::CheckArraysInheritTupleDims(dataStructure); } -TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - Feature array tuple count mismatch (-3020)", "[Core][CopyFeatureArrayToElementArrayFilter][preflight]") +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - Feature array tuple count mismatch (-3020)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") { UnitTest::LoadPlugins(); @@ -77,7 +194,7 @@ TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - CopyFeatureArrayToElementArrayFilter filter; Arguments args; - args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPath_Key, + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{DataPath({k_FeatureTemperatureName}), DataPath({k_FeatureDataArrayName})})); args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath({k_CellFeatureIdsArrayName}))); args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any(k_CellTempArraySuffix)); @@ -87,8 +204,273 @@ TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - REQUIRE(preflightResult.outputActions.errors()[0].code == -3020); } +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - Non-DataArray selection rejected", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + // A NeighborList is an IArray but NOT an IDataArray. The parameter restricts selections to + // ArrayType::DataArray, so this must fail parameter validation with a clean error instead of + // reaching preflightImpl() and throwing std::bad_cast from getDataRefAs(). + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureAMPath)); + auto& featureAM = dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureAMPath); + auto* neighborListPtr = NeighborList::Create(dataStructure, "NeighborList", featureAM.getShape(), featureAM.getId()); + REQUIRE(neighborListPtr != nullptr); + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, + std::make_any>(std::vector{AnalyticalFixtures::k_FeatureAMPath.createChildPath("NeighborList")})); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions) +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Preflight Error - Suffix contains '/' (-3021)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any("/Cell")); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions) + REQUIRE(preflightResult.outputActions.errors()[0].code == -3021); +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Execute Error - Created name collides with existing array (-266)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + // Selecting the FeatureIds array itself with an empty suffix derives a created path identical + // to the source path. IFilter::preflight() does not apply output actions, so the collision is + // reported when the CreateArrayAction is applied at execute (-266) — never a silent overwrite. + // (In a pipeline, the pipeline-level preflight applies actions and catches this before execute.) + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{AnalyticalFixtures::k_FeatureIdsPath})); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any("")); + + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_INVALID(executeResult.result) + REQUIRE(executeResult.result.errors()[0].code == -266); + + // The original FeatureIds array must be untouched. + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath)); + const auto& featureIdsStoreRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath).getDataStoreRef(); + for(usize i = 0; i < AnalyticalFixtures::k_FeatureIds.size(); i++) + { + CAPTURE(i); + REQUIRE(featureIdsStoreRef[i] == AnalyticalFixtures::k_FeatureIds[i]); + } +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Analytical Oracle (Class 1)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) + + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) + + const usize numCells = AnalyticalFixtures::k_FeatureIds.size(); + + // ------------------------------------------------------------------------ + // Class 1 (Analytical): compare against the hand-derived expected constants. + // out[i*C + c] = source[FeatureIds[i]*C + c] — derivation in the V&V report. + // ------------------------------------------------------------------------ + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_AvgTempCellPath)); + const auto& avgTempCellRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_AvgTempCellPath).getDataStoreRef(); + REQUIRE(avgTempCellRef.getNumberOfTuples() == numCells); + REQUIRE(avgTempCellRef.getNumberOfComponents() == 1); + for(usize i = 0; i < AnalyticalFixtures::k_ExpectedAvgTempCell.size(); i++) + { + CAPTURE(i); + REQUIRE(avgTempCellRef[i] == AnalyticalFixtures::k_ExpectedAvgTempCell[i]); + } + + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_RGBCellPath)); + const auto& rgbCellRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_RGBCellPath).getDataStoreRef(); + REQUIRE(rgbCellRef.getNumberOfTuples() == numCells); + REQUIRE(rgbCellRef.getNumberOfComponents() == AnalyticalFixtures::k_RGBComponentCount); + for(usize i = 0; i < AnalyticalFixtures::k_ExpectedRGBCell.size(); i++) + { + CAPTURE(i); + REQUIRE(rgbCellRef[i] == AnalyticalFixtures::k_ExpectedRGBCell[i]); + } + + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_ActiveCellPath)); + const auto& activeCellRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_ActiveCellPath).getDataStoreRef(); + REQUIRE(activeCellRef.getNumberOfTuples() == numCells); + REQUIRE(activeCellRef.getNumberOfComponents() == 1); + for(usize i = 0; i < AnalyticalFixtures::k_ExpectedActiveCell.size(); i++) + { + CAPTURE(i); + REQUIRE(activeCellRef[i] == AnalyticalFixtures::k_ExpectedActiveCell[i]); + } + + // ------------------------------------------------------------------------ + // Class 4 (Invariant): piecewise constancy — every pair of cells sharing a + // feature id must have identical output tuples (checked on the 3-comp array). + // ------------------------------------------------------------------------ + for(usize i = 0; i < numCells; i++) + { + for(usize j = i + 1; j < numCells; j++) + { + if(AnalyticalFixtures::k_FeatureIds[i] == AnalyticalFixtures::k_FeatureIds[j]) + { + for(usize c = 0; c < AnalyticalFixtures::k_RGBComponentCount; c++) + { + CAPTURE(i, j, c); + REQUIRE(rgbCellRef[i * AnalyticalFixtures::k_RGBComponentCount + c] == rgbCellRef[j * AnalyticalFixtures::k_RGBComponentCount + c]); + } + } + } + } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Execute Error - Negative FeatureIds (-5355)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + + // Corrupt one feature id to a negative value. Preflight cannot see array values, + // so this must pass preflight and fail in execute via + // ValidateFeatureIdsToFeatureAttributeMatrixIndexing (ignoreNegativeValues = false). + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath)); + auto& featureIdsStoreRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath).getDataStoreRef(); + featureIdsStoreRef[5] = -1; + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + + 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 == -5355); +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Execute Error - FeatureId exceeds Feature tuple count (-5351)", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = AnalyticalFixtures::CreateFixture(); + + // Corrupt one feature id to 4: the feature arrays have 4 tuples (valid ids 0-3), + // so id 4 would read past the end. Must pass preflight and fail in execute with -5351. + REQUIRE_NOTHROW(dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath)); + auto& featureIdsStoreRef = dataStructure.getDataRefAs(AnalyticalFixtures::k_FeatureIdsPath).getDataStoreRef(); + featureIdsStoreRef[5] = 4; + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args = AnalyticalFixtures::CreateArguments(); + + 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 == -5351); +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Over-provisioned Feature array accepted", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + // Pins deviation CopyFeatureArrayToElementArrayFilter-D2: DREAM3D 6.5.171 errors (-5555) when the + // feature array has more tuples than largestFeatureId + 1; SIMPLNX deliberately accepts it. + DataStructure dataStructure; + + // 6 cells referencing features 0-2; feature array over-provisioned with 8 tuples. + auto* featureIdsPtr = Int32Array::CreateWithStore>(dataStructure, k_CellFeatureIdsArrayName, {6}, {1}); + auto& featureIdsStoreRef = featureIdsPtr->getDataStoreRef(); + const std::vector featureIds = {0, 2, 1, 1, 0, 2}; + for(usize i = 0; i < featureIds.size(); i++) + { + featureIdsStoreRef[i] = featureIds[i]; + } + + auto* featureValuesPtr = Float32Array::CreateWithStore>(dataStructure, k_FeatureTemperatureName, {8}, {1}); + auto& featureValuesStoreRef = featureValuesPtr->getDataStoreRef(); + for(usize i = 0; i < featureValuesStoreRef.getNumberOfTuples(); i++) + { + featureValuesStoreRef[i] = static_cast(i) * 2.0F + 1.0F; // [1, 3, 5, 7, 9, 11, 13, 15] + } + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args; + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{DataPath({k_FeatureTemperatureName})})); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath({k_CellFeatureIdsArrayName}))); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any(k_CellTempArraySuffix)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) + + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) + + // Hand-derived: out[i] = featureValues[featureIds[i]] = [1, 5, 3, 3, 1, 5] + const DataPath createdPath({k_FeatureTemperatureName + k_CellTempArraySuffix}); + REQUIRE_NOTHROW(dataStructure.getDataRefAs(createdPath)); + const auto& createdRef = dataStructure.getDataRefAs(createdPath).getDataStoreRef(); + const std::vector expected = {1.0F, 5.0F, 3.0F, 3.0F, 1.0F, 5.0F}; + for(usize i = 0; i < expected.size(); i++) + { + CAPTURE(i); + REQUIRE(createdRef[i] == expected[i]); + } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} + +TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Zero-tuple FeatureIds accepted", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]") +{ + UnitTest::LoadPlugins(); + + // A FeatureIds array with zero tuples is degenerate but legal: there is nothing to copy, + // and the filter must succeed with empty output arrays (not crash in range validation). + DataStructure dataStructure; + + Int32Array::CreateWithStore>(dataStructure, k_CellFeatureIdsArrayName, {0}, {1}); + Float32Array::CreateWithStore>(dataStructure, k_FeatureTemperatureName, {4}, {1}); + + CopyFeatureArrayToElementArrayFilter filter; + Arguments args; + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{DataPath({k_FeatureTemperatureName})})); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath({k_CellFeatureIdsArrayName}))); + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any(k_CellTempArraySuffix)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) + + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) + + const DataPath createdPath({k_FeatureTemperatureName + k_CellTempArraySuffix}); + REQUIRE_NOTHROW(dataStructure.getDataRefAs(createdPath)); + const auto& createdRef = dataStructure.getDataRefAs(createdPath).getDataStoreRef(); + REQUIRE(createdRef.getNumberOfTuples() == 0); +} + using ListOfTypes = std::tuple; -TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Valid filter execution", "[Core][CopyFeatureArrayToElementArrayFilter]", ListOfTypes) +TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Valid filter execution", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter]", ListOfTypes) { UnitTest::LoadPlugins(); @@ -108,7 +490,8 @@ TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Vali } } - // Create a feature data array with 3 values + // Create two feature data arrays with 3 tuples each, filled with distinct per-feature + // values so that an indexing mistake in the filter cannot go undetected. DataArray* avgTempValuePtr = DataArray::template CreateWithStore>(dataStructure, k_FeatureTemperatureName, {3}, {1}); REQUIRE(avgTempValuePtr != nullptr); DataArray& avgTempValue = *avgTempValuePtr; @@ -116,16 +499,17 @@ TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Vali REQUIRE(featureDataPtr != nullptr); DataArray& featureDataValue = *featureDataPtr; - for(int i = 0; i < 3; i++) + for(usize i = 0; i < 3; i++) { - avgTempValue[i] = static_cast(0); + avgTempValue[i] = static_cast(i * 10 + 5); // [5, 15, 25] + featureDataValue[i] = static_cast(i * 3 + 1); // [1, 4, 7] } // Create filter and set arguments CopyFeatureArrayToElementArrayFilter filter; Arguments args; - args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPath_Key, + args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key, std::make_any>(std::vector{DataPath({k_FeatureTemperatureName}), DataPath({k_FeatureDataArrayName})})); args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(DataPath({k_CellFeatureIdsArrayName}))); args.insertOrAssign(CopyFeatureArrayToElementArrayFilter::k_CreatedArraySuffix_Key, std::make_any(k_CellTempArraySuffix)); @@ -139,11 +523,14 @@ TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Vali SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) // Check the filter results - auto& createdElementTempArray = dataStructure.getDataRefAs>(k_CellTempArrayPath); - auto& createdElementFeatureArray = dataStructure.getDataRefAs>(k_CellFeatureArrayPath); + REQUIRE_NOTHROW(dataStructure.getDataRefAs>(k_CellTempArrayPath)); + const auto& createdElementTempArray = dataStructure.getDataRefAs>(k_CellTempArrayPath); + REQUIRE_NOTHROW(dataStructure.getDataRefAs>(k_CellFeatureArrayPath)); + const auto& createdElementFeatureArray = dataStructure.getDataRefAs>(k_CellFeatureArrayPath); REQUIRE(createdElementTempArray.getNumberOfTuples() == createdElementFeatureArray.getNumberOfTuples()); for(usize i = 0; i < createdElementTempArray.getNumberOfTuples(); i++) { + CAPTURE(i); int32 featureId = cellFeatureIds[i]; TestType value1 = createdElementTempArray[i]; TestType value2 = createdElementFeatureArray[i]; @@ -152,6 +539,8 @@ TEMPLATE_LIST_TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: Vali REQUIRE(value1 == featureValue1); REQUIRE(value2 == featureValue2); } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); } TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: SIMPL Backwards Compatibility", "[SimplnxCore][CopyFeatureArrayToElementArrayFilter][BackwardsCompatibility]") @@ -187,7 +576,7 @@ TEST_CASE("SimplnxCore::CopyFeatureArrayToElementArrayFilter: SIMPL Backwards Co CHECK(pipelineFilter->getComments().empty()); const Arguments args = pipelineFilter->getArguments(); - // Complex type (SingleToMultiDataPathSelectionFilterParameterConverter) - verified by successful pipeline loading + CHECK(args.value>(CopyFeatureArrayToElementArrayFilter::k_SelectedFeatureArrayPaths_Key) == std::vector{DataPath({"DataContainer", "CellData", "TestArray"})}); CHECK(args.value(CopyFeatureArrayToElementArrayFilter::k_CellFeatureIdsArrayPath_Key) == DataPath({"DataContainer", "CellData", "TestArray"})); // The legacy CreatedArrayName is intentionally NOT mapped onto the suffix; the copied array keeps // the input array's name, so the suffix stays at its default (empty). diff --git a/src/Plugins/SimplnxCore/vv/CopyFeatureArrayToElementArrayFilter.md b/src/Plugins/SimplnxCore/vv/CopyFeatureArrayToElementArrayFilter.md new file mode 100644 index 0000000000..efdd1aa30e --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/CopyFeatureArrayToElementArrayFilter.md @@ -0,0 +1,142 @@ +# V&V Report: CopyFeatureArrayToElementArrayFilter + +| | | +|-----------------------------|------------------------------------------------------------------------------------------------------| +| Plugin | SimplnxCore | +| SIMPLNX UUID | `4c8c976a-993d-438b-bd8e-99f71114b9a1` | +| SIMPLNX Human Name | Create Element Array from Feature Array | +| DREAM3D 6.5.171 equivalent | `CopyFeatureArrayToElementArray` (SIMPLib CoreFilters) — SIMPL UUID `99836b75-144b-5126-b261-b411133b5e8a` | +| Verified commit | ** | +| Status | READY FOR REVIEW | +| Sign-off | *pending second-engineer review* | + +## At a glance + +| Aspect | Current state | +|-----------------------|---------------| +| Algorithm Relationship | **Minor changes** — same indirection-copy kernel as SIMPL `CopyFeatureArrayToElementArray`; NX deliberately adds multi-array selection, suffix-based output naming, TBB parallelization, and different feature-count validation semantics. | +| Oracle (confirmed) | **Class 1 (Analytical)** — pure indirection lookup `out[i*C+c] = feature[featureIds[i]*C+c]`; hand-derived expected values on a 4×3×1 fixture (float32/1-comp, int32/3-comp, bool). **Class 4 (Invariant)** companion — piecewise constancy within each feature. Encoded as `CopyFeatureArrayToElementArrayTest.cpp::"Analytical Oracle (Class 1)"`; all pass. | +| Code paths | 13 of 14 exercised; cancel-check path excluded by engineer instruction (no cancel-signal injection in test framework). | +| Tests | 21 ctest entries (12 TEST_CASEs, one a 10-type TEMPLATE_LIST): 1 analytical-oracle, 7 error/negative/degenerate, 1 deviation-pin, 10 type-dispatch, 1 SIMPL backwards-compat (2 DYNAMIC_SECTIONs). Both-build pass (see OOC caveat below). | +| Exemplar archive | None — all fixtures are in-memory `AnalyticalFixtures` built in test code; no `download_test_data()` archive required. | +| Legacy comparison | **Run 2026-07-23, re-run after post-review kernel rewrite.** Bit-identical numeric output on the main fixture (float32, int32×3, bool). 3 deviations, all naming/validation semantics: D1 (output naming for converted pipelines), D2 (over-provisioned feature array accepted in NX, error -5555 in legacy), D3 (negative ids: silent out-of-bounds garbage in legacy, hard error -5355 in NX). | +| Bug flags | `CopyFeatureArrayToElementArrayFilter-D3` — legacy 6.5.171 silently produces undefined values for negative feature ids (unchecked out-of-bounds read). SIMPLNX behavior is correct. | +| V&V phase | All standard phases complete, plus a five-review hardening pass (adversarial, senior-engineer, CPU-perf, memory-perf, OOC) whose Critical/Warning findings were fixed and re-verified. Outstanding: second-engineer review of oracle + report (pending); OOC-build backend gap (below) needs an infrastructure decision. | +| OOC caveat | The `simplnx-ooc-Rel` build sets `SIMPLNX_FORCE_OUT_OF_CORE_DATA=ON` but registers **no OOC backend** (`SIMPLNX_EXTRA_PLUGINS=FileStore` with an empty `SIMPLNX_FileStore_SOURCE_DIR`), so `useOocData()` is false and tests actually run in-core. "OOC pass" here certifies compile + run under the OOC configuration, **not** OOC data-path behavior. Applies to every filter tested from this build dir. | + +## Summary + +`CopyFeatureArrayToElementArrayFilter` copies each selected Feature-level array down to the Element (cell) level: for every cell `i`, the output tuple is the source array's tuple at index `featureIds[i]`. Verification used a Class 1 analytical oracle (hand-derived expected values for a 12-cell, 4-feature fixture across single- and multi-component arrays and three data types) plus Class 4 piecewise-constancy invariants; SIMPLNX matched the oracle exactly with zero discrepancies. A/B comparison against DREAM3D 6.5.171 was bit-identical on valid input, with 3 documented deviations in naming/validation semantics — including one legacy bug (silent undefined output for negative feature ids, D3). + +## Algorithm Relationship + +*Classification:* **Minor changes** + +*Evidence:* SIMPL UUID `99836b75-144b-5126-b261-b411133b5e8a` is registered for conversion via `FromSIMPLJson()` (fixtures at `test/simpl_conversion/6_5/` and `6_4/`). The copy kernel is semantically identical to legacy `copyData()` (SIMPL `CopyFeatureArrayToElementArray.cpp:159-191`) — a per-tuple indirection copy. The surrounding filter deliberately changed. + +*Port-time deltas:* + +1. **Multi-array selection** — legacy selects ONE feature array per filter instance; NX takes a `MultiArraySelectionParameter` and loops. Does not change per-array output values. +2. **Output naming** — legacy takes an explicit created-array *name*; NX builds the name as ``. Same numeric output, different output DataPath for converted pipelines (see Deviations D1). +3. **Feature-count validation relaxed** — legacy `execute()` errors (-5555) BOTH when `maxFeatureId >= numFeatures` AND when the feature array is over-provisioned (`maxFeatureId != numFeatures-1`). NX (`ValidateFeatureIdsToFeatureAttributeMatrixIndexing`, `ignoreNegativeValues=false`) errors only when `maxFeatureId >= numFeatures` (-5351); an over-sized feature array is accepted (see Deviations D2). +4. **Negative feature ids** — legacy performs an unchecked negative index into the feature array (undefined behavior / garbage read); NX errors with -5355 (see Deviations D3). +5. **Parallelization** — legacy is a serial `memcpy` per tuple; NX uses `ParallelDataAlgorithm` over cell tuples with per-component assignment. Element-wise independent writes; no accumulation, so no order-of-operations effect on output. +6. **Type dispatch** — legacy if/else `CanDynamicCast` chain over 11 types (bool + 8 int + 2 float); NX `ExecuteParallelFunction` with `ArrayUseAllTypes` over the same 11 types. No behavioral difference. +7. **Tuple-count precheck (new in NX)** — preflight requires all selected feature arrays to share a tuple count (error -3020). Legacy has no equivalent because it only ever operates on one array. + +*Material PRs since baseline:* #1644 (added -3020 preflight test), #1588 (SIMPL backwards-compat test redesign), #1544 (executeImpl → Algorithm class extraction). None changed the copy kernel. + +## Oracle + +*Class:* **1 (Analytical)** primary + **4 (Invariant)** companion. Classes 2/3/5 N/A — no external library or paper needed for an indirection lookup (this filter is the literal "Indirection lookups" example in `oracle_classes.md`). + +*Applied:* Hand-built 4×3×1 `ImageGeom` fixture, 12 cells, 4 features. `FeatureIds = [0,1,1,2, 2,0,3,1, 3,3,0,2]`. Feature arrays: `AvgTemp` (float32, 1 comp) `= [10.5, 20.25, -30.75, 40.125]` and `RGB` (int32, 3 comp) `= [(1,2,3), (40,50,60), (-7,8,-9), (100,200,127)]`. Expected cell outputs are derived by hand (spreadsheet-free — 12 lookups) and embedded as inline constants with derivation comments: + +- `AvgTemp_Cell = [10.5, 20.25, 20.25, -30.75, -30.75, 10.5, 40.125, 20.25, 40.125, 40.125, 10.5, -30.75]` +- `RGB_Cell` tuple `i` = RGB tuple `FeatureIds[i]`, e.g. cell 3 (fid 2) = `(-7,8,-9)`; cell 6 (fid 3) = `(100,200,127)`. + +Class 4 companion invariants: (a) every pair of cells with the same feature id has identical output tuples; (b) each output tuple equals the source feature tuple exactly (bit-identical, no arithmetic performed). + +*Encoded:* `test/CopyFeatureArrayToElementArrayTest.cpp::"Analytical Oracle (Class 1)"` — 12+36+12 element-wise Class 1 assertions (AvgTemp_Cell, RGB_Cell, Active_Cell) with the derivation embedded as comments, plus the Class 4 piecewise-constancy invariant loop. All pass in both builds. A secondary hand-derived fixture lives in `"Over-provisioned Feature array accepted"` (6 assertions). + +*Second-engineer review:* pending — requested as part of PR review of the test changes. + +## Code path coverage + +*13 of 14 paths exercised; cancel path excluded by engineer instruction.* + +Source: `src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/CopyFeatureArrayToElementArray.cpp` (139 lines) + preflight in `Filters/CopyFeatureArrayToElementArrayFilter.cpp`. + +Logical phases: (a) parameter/preflight validation + output-array creation, (b) execute-time validation, (c) type-dispatched copy kernel (raw-pointer fast path for in-core `DataStore`, virtual `AbstractDataStore` fallback otherwise). + +| # | Phase | Path | Test case | +|----|---------------|-------------------------------------------------------------------------------------------|-----------| +| 1 | (a) Preflight | empty selection list → error `k_Validate_Empty_Value` (filter's own guard, preflight AND execute) | `Preflight Error - Empty selection (filter guard)` — FeatureIds path is valid so parameter validation passes and the filter guard is what fires | +| 2 | (a) Preflight | non-`IDataArray` selection (NeighborList/StringArray) → rejected by parameter validation (`ArrayType::DataArray` constraint) | `Preflight Error - Non-DataArray selection rejected` | +| 3 | (a) Preflight | suffix contains `/` → error -3021 | `Preflight Error - Suffix contains '/' (-3021)` | +| 4 | (a) Preflight | selected feature arrays disagree on tuple count → error -3020 | `Preflight Error - Feature array tuple count mismatch (-3020)` | +| 5 | (a) Preflight | valid → one `CreateArrayAction` per selected array (tuple shape from FeatureIds, component shape from source, name ``) | `Analytical Oracle (Class 1)` — 3 arrays created in the Cell AM | +| 6 | (a) Actions | created path collides with an existing object → error -266 at action application (execute in unit tests; pipeline preflight in the GUI); original array untouched | `Execute Error - Created name collides with existing array (-266)` | +| 7 | (b) Execute | negative feature id → error -5355 (`ValidateFeatureIdsToFeatureAttributeMatrixIndexing`, called once — preflight -3020 guarantees equal tuple counts across selected arrays) | `Execute Error - Negative FeatureIds (-5355)` | +| 8 | (b) Execute | `maxFeatureId >= numFeatures` → error -5351 | `Execute Error - FeatureId exceeds Feature tuple count (-5351)` | +| 9 | (b) Execute | over-provisioned feature array (`numFeatures > maxId+1`) → accepted (deviation D2 pin) | `Over-provisioned Feature array accepted` | +| 10 | (b) Execute | zero-tuple FeatureIds → valid no-op (validator's empty guard), empty outputs | `Zero-tuple FeatureIds accepted` | +| 11 | (b) Execute | cancel check (per-array loop + inside both kernel paths) | *Not directly tested. Excluded by engineer instruction — requires cancel-signal injection.* | +| 12 | (c) Kernel | 11-way type dispatch (bool + 8 int + 2 float) | TEMPLATE_LIST `Valid filter execution` (10 numeric); `Analytical Oracle (Class 1)` (bool, float32, int32) | +| 13 | (c) Kernel | raw-pointer fast path (all three stores are in-core `DataStore`) + multi-component copy (`C > 1`) | `Analytical Oracle (Class 1)` (in-core build) — RGB 3-comp; all in-core tests take this path | +| 14 | (c) Kernel | virtual `AbstractDataStore` fallback (non-`DataStore` stores, e.g. out-of-core) | Exercised only when an OOC backend is registered (see OOC caveat); logic is the pre-rewrite kernel unchanged. *Partially covered pending OOC build fix.* | + +## Test inventory + +| Test case | Status | Notes | +|-----------|--------|-------| +| `Preflight Error - Empty selection (filter guard)` | new-for-V&V (replaces `Parameter Check`) | Adversarial review showed the old test's empty FeatureIds path failed *parameter* validation before `preflightImpl` ran, leaving the filter's own guard untested. Now uses a valid FeatureIds array so the guard itself fires (preflight + execute). | +| `Preflight Error - Feature array tuple count mismatch (-3020)` | kept | Two feature arrays with 3 vs 4 tuples → -3020. | +| `Preflight Error - Non-DataArray selection rejected` | new-for-V&V | Pins the `ArrayType::DataArray` parameter constraint added after the adversarial review (selecting a NeighborList previously threw an uncaught `std::bad_cast` from preflight). | +| `Preflight Error - Suffix contains '/' (-3021)` | new-for-V&V | Pins the suffix validation added after the adversarial review (previously threw uncaught `std::invalid_argument` from `DataObject` name validation). | +| `Execute Error - Created name collides with existing array (-266)` | new-for-V&V | Empty suffix + self-selection → -266 at action application; asserts the source array is untouched (no silent overwrite). | +| `Analytical Oracle (Class 1)` | new-for-V&V | ImageGeom fixture; 60 element-wise Class 1 assertions vs hand-derived constants (float32/1, int32/3, bool/1) + Class 4 piecewise-constancy loop + `CheckArraysInheritTupleDims`. | +| `Execute Error - Negative FeatureIds (-5355)` | new-for-V&V | Passes preflight, fails execute; pins deviation D3's SIMPLNX side. | +| `Execute Error - FeatureId exceeds Feature tuple count (-5351)` | new-for-V&V | id 4 vs 4-tuple feature array. | +| `Over-provisioned Feature array accepted` | new-for-V&V | 8-tuple feature array, max id 2 → success + 6 hand-derived value assertions; pins deviation D2. | +| `Zero-tuple FeatureIds accepted` | new-for-V&V | Degenerate no-op case; also pins the empty-store guard added to `ValidateFeatureIdsToFeatureAttributeMatrixIndexing` (previously dereferenced `end()` — UB). | +| `Valid filter execution` (TEMPLATE_LIST ×10 types) | kept (modified) | Was comparing never-initialized feature data (indeterminate values) and all-zero temperature data — an indexing bug could not have been detected. Now initialized with distinct per-feature values `[5,15,25]` / `[1,4,7]`. Added `REQUIRE_NOTHROW`, `CAPTURE(i)`, `CheckArraysInheritTupleDims`. | +| `SIMPL Backwards Compatibility` (2 DYNAMIC_SECTIONs) | kept (modified) | UUID + argument conversion round-trip for 6.4 and 6.5 fixtures; now also asserts the converted multi-path selection value (was previously unasserted). | + +All 21 ctest entries pass in both `simplnx-Rel` (in-core) and `simplnx-ooc-Rel` builds, 2026-07-23 (see OOC caveat in At a glance). + +## Post-V&V hardening (five-review pass, 2026-07-23) + +Five independent reviews were run after the standard workflow: adversarial, nit-picky senior engineer, CPU performance, memory performance, and out-of-core. Actions taken: + +**Fixed (code):** +1. *(Adversarial, Critical)* Selecting a NeighborList/StringArray crashed preflight with uncaught `std::bad_cast` — parameter now constrained to `ArrayType::DataArray`; pinned by test. +2. *(Adversarial)* The empty-selection guard was untested (old `Parameter Check` failed on the FeatureIds parameter before reaching the guard) — test reworked to exercise the guard itself. +3. *(Adversarial)* Suffix containing `/` threw uncaught `std::invalid_argument` — preflight now rejects with -3021; pinned by test. +4. *(Adversarial)* Zero-tuple FeatureIds dereferenced `end()` (UB) in `ValidateFeatureIdsToFeatureAttributeMatrixIndexing` — empty guard added in the shared utility; pinned by test. +5. *(CPU/Memory/OOC, consensus)* FeatureIds min/max validation ran once per selected array (K full array scans) — hoisted to a single call (valid because preflight -3020 guarantees equal tuple counts). +6. *(CPU perf)* Copy kernel did ~3 virtual calls per component-element via `AbstractDataStore`/`ValueProxy` (~10-30× slower per thread than the legacy memcpy gather) — added a raw-pointer fast path taken when all three stores are concrete in-core `DataStore` (`std::copy_n` per tuple, cancel check every 65536 tuples), with the virtual path kept as the OOC/other-store fallback. This also resolves the thread-safety review question: parallel threads write disjoint ranges of a raw buffer on the fast path; non-`DataStore` stores fall back to the virtual path, and `requireArraysInMemory()` + the OOC auto-serialization keep that path serial. +7. *(Senior)* Naming/style/test-hygiene items: `k_SelectedFeatureArrayPaths_Key` rename (JSON key unchanged), `Xmdf`→`Xdmf`, `tDims`→`tupleShape`, `p...Value` prefix consistency, const `InputValues` ctor param, doxygen rewrite, `REQUIRE_NOTHROW` before every `getDataRefAs`, `CAPTURE(i)` in assertion loops, tag standardization to `[SimplnxCore]`, magic-number cleanup, removed unused ``, backcompat test now asserts the converted multi-path value. +8. *(Memory/docs)* Docs now state the per-array Element-sized memory cost and the -3020/-3021 error codes. + +**Corrected finding:** the adversarial review predicted the name-collision case would error at *preflight* (-266); empirically `IFilter::preflight` does not apply output actions, so the error surfaces at execute (or pipeline-level preflight in the GUI). The test pins the actual behavior and that the source array is untouched. + +**Deferred / escalated:** +- *(OOC, Critical — infrastructure, not this filter)* The `simplnx-ooc-Rel` build registers no OOC backend, so its "OOC" test runs are actually in-core (see At a glance caveat). Needs a build-infrastructure decision (FileStore source dir vs SimplnxOoc wiring). +- *(OOC)* Slab/bulk-I/O restructure and feature-array local caching for the OOC rewrite branch's chunked stores — deferred to the OOC architecture rewrite, where `copyIntoBuffer`/`copyFromBuffer` exist. +- *(OOC)* Tier-1 200³ OOC test — deferred until the OOC backend gap is fixed (it would prove nothing today). +- *(Memory, framework-wide)* `CreateArrayAction` zero-fills every created store before the kernel overwrites 100% of it (double touch) — framework enhancement request, not filter-specific. +- *(Adversarial, framework-wide)* `MultiArraySelectionParameter` stores but never enforces `AllowedDataTypes`; the `ArrayType` constraint is what protects this filter. + +## Exemplar archive + +- **Archive:** None. All fixtures are built in-memory in the test file (`AnalyticalFixtures`); no exemplar `.dream3d`/`download_test_data()` entry exists or is needed for this filter. +- **Provenance:** N/A (no archive). The A/B comparison inputs are minted by `make_input.py` in the comparison bundle (OneDrive archive), reproducible from the checked-in oracle constants. + +## Deviations from DREAM3D 6.5.171 + +Comparison run 2026-07-23 on the analytical fixture (main case) plus two targeted probes. **Numeric output bit-identical on valid input** (float32/1-comp, int32/3-comp, bool arrays). + +- `CopyFeatureArrayToElementArrayFilter-D1` — converted legacy pipelines produce differently *named* output arrays (`` vs explicit legacy name) — see `vv/deviations/CopyFeatureArrayToElementArrayFilter.md` +- `CopyFeatureArrayToElementArrayFilter-D2` — over-provisioned feature array errors in 6.5.171 (-5555) but succeeds in SIMPLNX — see `vv/deviations/CopyFeatureArrayToElementArrayFilter.md` +- `CopyFeatureArrayToElementArrayFilter-D3` — **legacy bug**: negative feature ids silently produce undefined values in 6.5.171 (unchecked out-of-bounds read); SIMPLNX errors -5355 — see `vv/deviations/CopyFeatureArrayToElementArrayFilter.md` diff --git a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_nx_pipeline.py b/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_nx_pipeline.py deleted file mode 100644 index 0878f39c3c..0000000000 --- a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_nx_pipeline.py +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/env python3 -"""Generate an NX .d3dpipeline: Read input -> RotateSampleRefFrame (in-place) -> Write output.""" -import json -import sys - -WORK = "/private/tmp/claude-501/-Users-mjackson-Workspace3-simplnx/8f4ca080-1346-435d-9929-f3a94423bf95/scratchpad/rotate_vv" -IDENTITY_4x4 = [[1.0, 0.0, 0.0, 0.0], [0.0, 1.0, 0.0, 0.0], [0.0, 0.0, 1.0, 0.0], [0.0, 0.0, 0.0, 1.0]] - - -def make(case, axis_angle): - inp = f"{WORK}/input_rotate.dream3d" - out = f"{WORK}/output_nx_{case}.dream3d" - pipeline = { - "name": f"rotate_nx_{case}", - "pipeline": [ - { - "args": {"import_data_object": {"value": {"data_paths": [], "file_path": inp, "path_import_policy": 0}, "version": 2}, "parameters_version": 1}, - "comments": "", "filter": {"name": "nx::core::ReadDREAM3DFilter", "uuid": "0dbd31c7-19e0-4077-83ef-f4a6459a0e2d"}, "isDisabled": False, - }, - { - "args": { - "input_image_geometry_path": {"value": "ImageDataContainer", "version": 1}, - "output_image_geometry_path": {"value": "ImageDataContainer", "version": 1}, - "remove_original_geometry": {"value": True, "version": 1}, - "keep_input_geometry_origin": {"value": False, "version": 1}, - "rotate_slice_by_slice": {"value": False, "version": 1}, - "rotation_representation_index": {"value": 0, "version": 1}, - "rotation_axis_angle": {"value": axis_angle, "version": 1}, - "rotation_matrix": {"value": IDENTITY_4x4, "version": 1}, - "parameters_version": 1, - }, - "comments": "", "filter": {"name": "nx::core::RotateSampleRefFrameFilter", "uuid": "d2451dc1-a5a1-4ac2-a64d-7991669dcffc"}, "isDisabled": False, - }, - { - "args": {"export_file_path": {"value": out, "version": 1}, "write_xdmf_file": {"value": False, "version": 1}, "use_compression": {"value": False, "version": 1}, "compression_level": {"value": 1, "version": 1}, "parameters_version": 1}, - "comments": "", "filter": {"name": "nx::core::WriteDREAM3DFilter", "uuid": "b3a95784-2ced-41ec-8d3d-0242ac130003"}, "isDisabled": False, - }, - ], - "version": 1, - } - path = f"{WORK}/nx_{case}.d3dpipeline" - json.dump(pipeline, open(path, "w"), indent=2) - print("wrote", path) - - -CASES = {"90Z": [0.0, 0.0, 1.0, 90.0], "180Z": [0.0, 0.0, 1.0, 180.0], "90X": [1.0, 0.0, 0.0, 90.0], "180Y": [0.0, 1.0, 0.0, 180.0]} -if __name__ == "__main__": - for c, aa in CASES.items(): - make(c, aa) diff --git a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_simpl_pipeline.py b/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_simpl_pipeline.py deleted file mode 100644 index 818497bc49..0000000000 --- a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/gen_simpl_pipeline.py +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env python3 -"""Generate legacy SIMPL .json pipelines: DataContainerReader -> RotateSampleRefFrame -> DataContainerWriter.""" -import json - -WORK = "/private/tmp/claude-501/-Users-mjackson-Workspace3-simplnx/8f4ca080-1346-435d-9929-f3a94423bf95/scratchpad/rotate_vv" -INP = f"{WORK}/input_rotate.dream3d" - - -def make(case, axis, angle): - out = f"{WORK}/output_6_5_171_{case}.dream3d" - pipe = { - "0": { - "FilterVersion": "6.5.171", "Filter_Enabled": True, "Filter_Human_Label": "Read DREAM.3D Data File", - "Filter_Name": "DataContainerReader", "Filter_Uuid": "{043cbde5-3878-5718-958f-ae75714df0df}", - "InputFile": INP, "OverwriteExistingDataContainers": False, - "InputFileDataContainerArrayProxy": { - "Data Containers": [{ - "Attribute Matricies": [{ - "Data Arrays": [{ - "Component Dimensions": [1], "Flag": 2, "Name": "Data", "Object Type": "DataArray", - "Path": "/DataContainers/ImageDataContainer/CellData", "Tuple Dimensions": [1], "Version": 2, - }], - "Flag": 2, "Name": "CellData", "Type": 3, - }], - "Flag": 2, "Geometry": {"Geometry_Type": 0, "Geometry_Type_Name": "ImageGeometry"}, "Name": "ImageDataContainer", - }], - "Version": 6, - }, - }, - "1": { - "FilterVersion": "6.5.171", "Filter_Enabled": True, "Filter_Human_Label": "Rotate Sample Reference Frame", - "Filter_Name": "RotateSampleRefFrame", "Filter_Uuid": "{e25d9b4c-2b37-578c-b1de-cf7032b5ef19}", - "RotationAngle": float(angle), "RotationAxis": {"x": float(axis[0]), "y": float(axis[1]), "z": float(axis[2])}, - "CellAttributeMatrixPath": {"Data Container Name": "ImageDataContainer", "Attribute Matrix Name": "CellData", "Data Array Name": ""}, - }, - "2": { - "FilterVersion": "6.5.171", "Filter_Enabled": True, "Filter_Human_Label": "Write DREAM.3D Data File", - "Filter_Name": "DataContainerWriter", "Filter_Uuid": "{3fcd4c43-9d75-5b86-aad4-4441bc914f37}", - "OutputFile": out, "WriteTimeSeries": False, "WriteXdmfFile": False, - }, - "PipelineBuilder": {"Name": f"legacy_rotate_{case}", "Number_Filters": 3, "Version": 6}, - } - path = f"{WORK}/simpl_{case}.json" - json.dump(pipe, open(path, "w"), indent=2) - print("wrote", path) - - -CASES = {"90Z": ((0, 0, 1), 90), "180Z": ((0, 0, 1), 180), "90X": ((1, 0, 0), 90), "180Y": ((0, 1, 0), 180)} -if __name__ == "__main__": - for c, (ax, ang) in CASES.items(): - make(c, ax, ang) diff --git a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/make_input.py b/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/make_input.py deleted file mode 100644 index e8e8420317..0000000000 --- a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/make_input.py +++ /dev/null @@ -1,30 +0,0 @@ -#!/usr/bin/env python3 -"""Mint the shared A/B input for RotateSampleRefFrame legacy comparison. - -4x3x2 ImageGeom (nx=4, ny=3, nz=2), origin (0,0,0), spacing (1,1,1), with an -Int32 "Data" cell array filled 1..24 in ZYX (slowest-to-fastest) order. Distinct, -nonzero values so the exact voxel permutation produced by each version is visible. -""" -import sys -import os -import numpy as np - -sys.path.insert(0, "/Users/mjackson/Workspace1/Claude_Support/skills/compare-legacy-dream3d") -from legacy_dream3d import D3DLegacyWriter - -NX, NY, NZ = 4, 3, 2 -N = NX * NY * NZ # 24 - -data = np.arange(1, N + 1, dtype=np.int32) # 1..24, ZYX order - - -def build(out_path): - with D3DLegacyWriter(out_path) as w: - w.add_image_geom("ImageDataContainer", dims=(NX, NY, NZ), origin=(0.0, 0.0, 0.0), spacing=(1.0, 1.0, 1.0)) - w.add_attribute_matrix("ImageDataContainer", "CellData", (NZ, NY, NX), "Cell") - w.add_data_array("ImageDataContainer", "CellData", "Data", data) - print("wrote", out_path) - - -if __name__ == "__main__": - build(sys.argv[1] if len(sys.argv) > 1 else "input_rotate.dream3d") diff --git a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/results.md b/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/results.md deleted file mode 100644 index 34afa07ac2..0000000000 --- a/src/Plugins/SimplnxCore/vv/comparisons/RotateSampleRefFrameFilter/results.md +++ /dev/null @@ -1,38 +0,0 @@ -# Legacy Comparison: RotateSampleRefFrame - -Date: 2026-07-02 - -## Environment - -- **DREAM3D 6.5.171:** `/Users/mjackson/Applications/DREAM3D.app/Contents/Bin/PipelineRunner` (official release) -- **DREAM3D-NX:** `.../DREAM3D-Build/NX-Com-Qt69-Vtk96-Rel/Bin/nxrunner` -- **Legacy filter:** `Source/Plugins/Sampling/SamplingFilters/RotateSampleRefFrame.cpp` (SIMPL UUID `{e25d9b4c-2b37-578c-b1de-cf7032b5ef19}`) - -## Input Data - -Shared, identical input minted with the `legacy_dream3d` h5py writer (`make_input.py`): a `4×3×2` **Image Geometry** (`ImageDataContainer`), origin (0,0,0), spacing (1,1,1), with an Int32 cell array `Data` filled `1..24` in ZYX order (distinct, nonzero → the exact voxel permutation is visible). FileVersion `7.0`, read correctly by both PipelineRunner and nxrunner. See `make_input.py`, `gen_simpl_pipeline.py`, `gen_nx_pipeline.py`. - -Both versions rotate in place (`ImageDataContainer/CellData/Data`). Legacy has no slice-by-slice or representation parameter (always full 3D rotation); NX configured with the Axis-Angle representation, `remove_original_geometry=true`, `slice_by_slice=false`. - -## Test Cases and Results - -| Case | Axis-Angle | Output dims (nx,ny,nz) | Legacy vs SIMPLNX | -|---|---|---|---| -| 90Z | (0,0,1,90) | (3,4,2) | **bit-identical** | -| 180Z | (0,0,1,180) | (4,3,2) | **bit-identical** | -| 90X | (1,0,0,90) | (4,2,3) | **bit-identical** | -| 180Y | (0,1,0,180) | (4,3,2) | **bit-identical** | - -All four cases: identical output dimensions **and** identical voxel values (element-wise equal, 0/24 differ). SIMPLNX also matches the independent Class 1 analytical-permutation oracle encoded in the unit test (e.g. 180Z reverses each Z slice: `[12..1, 24..13]`). - -## Root-cause note - -Legacy computes the source index by **truncation** (`colOld = (int64)(coord/xRes)`, old-origin assumed 0) while SIMPLNX inverse-maps each **cell center** through origin-aware `ImageGeom::computeCellIndex`. For an exact 90°-multiple rotation about a principal axis with integer coordinates and origin (0,0,0), every output cell center inverse-maps exactly onto a source cell center, so truncation and nearest-cell resolve to the same source voxel — the two implementations converge exactly. (Off-axis / non-90 rotations, where the two rules would diverge, are now rejected in SIMPLNX preflight; see deviation D1.) - -## Fixes Applied - -None — outputs matched on the entire supported (principal-90) domain. - -## Deviation - -One documented behavioral deviation, `RotateSampleRefFrameFilter-D1`: legacy silently accepts arbitrary (non-principal-90) rotations and produces a lossy nearest-neighbor resample; SIMPLNX rejects them in preflight (`-6850`). Intentional (root cause: algorithmic choice). See `../../deviations/RotateSampleRefFrameFilter.md`. diff --git a/src/Plugins/SimplnxCore/vv/deviations/CopyFeatureArrayToElementArrayFilter.md b/src/Plugins/SimplnxCore/vv/deviations/CopyFeatureArrayToElementArrayFilter.md new file mode 100644 index 0000000000..c3e1f9388e --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/deviations/CopyFeatureArrayToElementArrayFilter.md @@ -0,0 +1,65 @@ +# Deviations from DREAM3D 6.5.171: CopyFeatureArrayToElementArrayFilter + +This file lists every documented behavioral difference between this SIMPLNX filter and its DREAM3D 6.5.171 equivalent (`CopyFeatureArrayToElementArray`, SIMPL UUID `99836b75-144b-5126-b261-b411133b5e8a`). + +Entries are referenced by stable ID (`CopyFeatureArrayToElementArrayFilter-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. + +**Numeric output on valid input is bit-identical** — A/B comparison run 2026-07-23 on a 4×3×1 / 4-feature fixture with float32 (1-comp), int32 (3-comp), and bool arrays showed all output arrays bit-identical between 6.5.171 and SIMPLNX. All deviations below concern *naming* and *input-validation* behavior, not numeric results. + +--- + +## CopyFeatureArrayToElementArrayFilter-D1 + +| Field | Value | +|---|---| +| **Deviation ID** | `CopyFeatureArrayToElementArrayFilter-D1` | +| **Filter UUID** | `4c8c976a-993d-438b-bd8e-99f71114b9a1` | +| **Status** | active | + +**Symptom:** Pipelines converted from legacy DREAM3D produce output arrays with different *names*: legacy names the created array exactly what the user typed (e.g., `MyOutput`); SIMPLNX names it `` (the legacy name string is converted into the suffix, so a legacy pipeline copying `EquivalentDiameters` to `MyOutput` produces `EquivalentDiametersMyOutput` in SIMPLNX). + +**Root cause:** Algorithmic choice. The SIMPLNX filter was redesigned from single-array-with-explicit-name to multi-array-with-shared-suffix (`MultiArraySelectionParameter` + `created_array_suffix`). `FromSIMPLJson()` maps legacy `CreatedArrayName` onto the suffix (`CopyFeatureArrayToElementArrayFilter.cpp`, `FromSIMPLJson`), which cannot reproduce the legacy naming for a non-empty name. + +**Affected users:** Anyone importing a legacy `.json` pipeline containing this filter. Downstream filters in the converted pipeline that reference the legacy output-array name will fail preflight until the user updates the reference (or renames via a Rename Data Object filter). + +**Recommendation:** Trust SIMPLNX. Numeric content is identical; after conversion, update downstream array references to the new `` name. + +--- + +## CopyFeatureArrayToElementArrayFilter-D2 + +| Field | Value | +|---|---| +| **Deviation ID** | `CopyFeatureArrayToElementArrayFilter-D2` | +| **Filter UUID** | `4c8c976a-993d-438b-bd8e-99f71114b9a1` | +| **Status** | active | + +**Symptom:** When the selected Feature array has more tuples than `largestFeatureId + 1` (an over-provisioned Feature AttributeMatrix), DREAM3D 6.5.171 aborts with error -5555 ("The number of Features in the InArray array (N) does not match the largest Feature Id"); SIMPLNX runs successfully. + +**Root cause:** Algorithmic choice. Legacy `execute()` required `largestFeatureId == numFeatures - 1` exactly (SIMPL `CopyFeatureArrayToElementArray.cpp:235-241`). SIMPLNX (`ValidateFeatureIdsToFeatureAttributeMatrixIndexing`, `DataArrayUtilities.cpp:160-198`) only rejects ids that would index *past* the array (`maxFeatureId >= numFeatures`, error -5351). Extra unused feature tuples are legal in SIMPLNX — they are simply never read. + +Confirmed by A/B probe 2026-07-23: 6-cell fixture with max id 2 and an 8-tuple feature array — 6.5.171 errored -5555; SIMPLNX produced correct output for all referenced tuples (verified against hand-derived values; also pinned by unit test `Over-provisioned Feature array accepted`). + +**Affected users:** Users whose Feature AttributeMatrix retains tuples for features no longer present in the FeatureIds map (e.g., after feature removal/cropping). Their legacy pipelines failed; the same data now processes in SIMPLNX. + +**Recommendation:** Trust SIMPLNX. The legacy strict-equality check rejected valid input; the SIMPLNX output for all referenced feature ids is well-defined and verified. + +--- + +## CopyFeatureArrayToElementArrayFilter-D3 + +| Field | Value | +|---|---| +| **Deviation ID** | `CopyFeatureArrayToElementArrayFilter-D3` | +| **Filter UUID** | `4c8c976a-993d-438b-bd8e-99f71114b9a1` | +| **Status** | active | + +**Symptom:** When the FeatureIds array contains a negative value, DREAM3D 6.5.171 **completes without any error** and writes an undefined value at the affected cells; SIMPLNX stops with error -5355 ("Feature Ids array … has negative values"). + +**Root cause:** Bug in 6.5.171. Legacy `copyData()` computes `fPtr + (numComp * featureIdx)` with no lower-bound check (SIMPL `CopyFeatureArrayToElementArray.cpp:178-189`); a negative id reads out of bounds before the feature array's allocation — undefined behavior whose result depends on adjacent heap contents. The legacy max-id scan (`m_FeatureIds[i] > largestFeature`) only tracks the maximum, so negatives pass validation silently. SIMPLNX validates the minimum feature id before copying and refuses to run. + +Confirmed by A/B probe 2026-07-23: fixture with `FeatureIds[5] = -1` — 6.5.171 reported "Pipeline Complete" and wrote 0.0 (whatever bytes preceded the allocation) at cell 5; SIMPLNX errored -5355 and produced no output. + +**Affected users:** Anyone whose FeatureIds contains negative sentinel values (some workflows use -1 for "unassigned"). Legacy results at those cells were silent garbage. + +**Recommendation:** Trust SIMPLNX. The legacy behavior is undefined and platform-dependent; SIMPLNX's hard error is the correct response. Users with -1 sentinels must clean/reassign them before copying feature data down to cells. diff --git a/src/simplnx/Utilities/DataArrayUtilities.cpp b/src/simplnx/Utilities/DataArrayUtilities.cpp index 24c3349962..00b250a6ba 100644 --- a/src/simplnx/Utilities/DataArrayUtilities.cpp +++ b/src/simplnx/Utilities/DataArrayUtilities.cpp @@ -163,7 +163,6 @@ Result<> ValidateFeatureIdsToFeatureAttributeMatrixIndexing(const DataStructure& messageHandler(IFilter::ProgressMessage{IFilter::ProgressMessage::Type::Info, fmt::format("Validating range of values within input array '{}'...", featureIds.getName())}); usize numFeatures = 0; - std::string sourceName; // Check if an Attribute Matrix was passed in auto* targetAttributeMatrixPtr = dataStructure.getDataAs(sourceDataPath); @@ -179,6 +178,12 @@ Result<> ValidateFeatureIdsToFeatureAttributeMatrixIndexing(const DataStructure& } auto& featureIdsStore = featureIds.getDataStoreRef(); + if(featureIdsStore.getNumberOfTuples() == 0) + { + // Nothing to validate (and minmax_element over an empty store would dereference end()) + return {}; + } + auto [minFeatureId, maxFeatureId] = std::minmax_element(featureIdsStore.begin(), featureIdsStore.end()); if(!ignoreNegativeValues && *minFeatureId < 0)