diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/MultiThresholdObjects.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/MultiThresholdObjects.cpp index 48ea54b9d7..4a7aa094a8 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/MultiThresholdObjects.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/MultiThresholdObjects.cpp @@ -3,61 +3,148 @@ #include "simplnx/Common/TypeTraits.hpp" #include "simplnx/DataStructure/DataArray.hpp" #include "simplnx/Utilities/ArrayThreshold.hpp" +#include "simplnx/Utilities/DataStoreUtilities.hpp" #include "simplnx/Utilities/FilterUtilities.hpp" -#include +#include using namespace nx::core; namespace { -template +/** + * @brief InsertThreshold is used by ThresholdSets to apply their values to the parent collection using the appropriate union operator and + * inversion of true/false values. + * @param currentVector Accumulator for the set being built. + * @param unionOperator Union operator to combine with. Ignored when replaceOutput is true. + * @param newVector Values to apply to the accumulator. + * @param inverse Flip each incoming value before applying it. + * @param replaceOutput Overwrite the accumulator instead of combining with it. Set for the first child of a set, + * which has nothing to combine with yet; this is what lets the accumulator skip being pre-filled. + */ +void InsertThreshold(AbstractDataStore& currentVector, nx::core::IArrayThreshold::UnionOperator unionOperator, const AbstractDataStore& newVector, bool inverse, bool replaceOutput) +{ + usize numItems = currentVector.getNumberOfTuples(); + + // The union operator, the inversion flag, and the replace flag are the same for every tuple, so branch on + // them once here rather than once per element. Comparing the incoming value against 'inverse' flips it when + // inversion is requested without a branch inside the loop. + if(replaceOutput) + { + for(usize i = 0; i < numItems; i++) + { + currentVector.setValue(i, newVector.getValue(i) != inverse); + } + } + else if(nx::core::IArrayThreshold::UnionOperator::Or == unionOperator) + { + for(usize i = 0; i < numItems; i++) + { + currentVector.setValue(i, currentVector.getValue(i) || (newVector.getValue(i) != inverse)); + } + } + else + { + for(usize i = 0; i < numItems; i++) + { + currentVector.setValue(i, currentVector.getValue(i) && (newVector.getValue(i) != inverse)); + } + } +} + +/** + * @brief Consolidate all assignment calls to a single method to prevent unintended diverging behavior. + * @param arrayThreshold Current threshold to pull settings from. + * @param outputResultStore Output DataStore for the current ThresholdSet. + * @param inputThresholdStore Resulting output for the target array threshold. + * @param replaceInput The first threshold in every set has its output applied to the output regardless of union operator. + */ +void ApplyThresholdValues(const IArrayThreshold& arrayThreshold, AbstractDataStore& outputResultStore, const AbstractDataStore& inputThresholdStore, bool replaceInput) +{ + // insert into current threshold + InsertThreshold(outputResultStore, arrayThreshold.getUnionOperator(), inputThresholdStore, arrayThreshold.isInverted(), replaceInput); +} + class ThresholdFilterHelper { public: - ThresholdFilterHelper(ArrayThreshold::ComparisonType compType, ArrayThreshold::ComparisonValue compValue, usize componentIndex, std::vector& output) + ThresholdFilterHelper(ArrayThreshold::ComparisonType compType, ArrayThreshold::ComparisonValue compValue, usize componentIndex, IArrayThreshold::UnionOperator unionType, + AbstractDataStore& output, bool invert, bool replaceOutput) : m_ComparisonOperator(compType) , m_ComparisonValue(compValue) , m_ComponentIndex(componentIndex) + , m_UnionType(unionType) , m_Output(output) + , m_Invert(invert) + , m_ReplaceOutput(replaceOutput) { } template - void filterDataWithComparision(const AbstractDataStore& m_Input, T trueValue, T falseValue) + void filterDataWithComparision(const AbstractDataStore& inputStore) { - size_t numTuples = m_Input.getNumberOfTuples(); - T value = static_cast(m_ComparisonValue); - for(size_t tupleIndex = 0; tupleIndex < numTuples; ++tupleIndex) + usize numTuples = inputStore.getNumberOfTuples(); + // The comparison value is truncated to the input array's type and the comparison is performed in that + // type. This matches legacy DREAM3D (SIMPL ThresholdFilterHelper), where a threshold of 5.5 against an + // int32 array compares against 5, and keeps 64-bit integer comparisons exact. + const T comparisonValue = static_cast(m_ComparisonValue); + + // The union operator and the invert flag are the same for every tuple, so they are resolved once here + // instead of inside the loop. m_Output holds a single component per tuple throughout. + if(m_ReplaceOutput) { - T inputValue = m_Input.getComponentValue(tupleIndex, m_ComponentIndex); - T outputValue = CompT{}(inputValue, value) ? trueValue : falseValue; - m_Output[tupleIndex] = outputValue; + // First threshold in a set: nothing to combine with, so the result is written straight out. This is + // also why the accumulator does not need to be pre-filled with FALSE. + for(usize tupleIndex = 0; tupleIndex < numTuples; ++tupleIndex) + { + m_Output.setValue(tupleIndex, CompT{}(inputStore.getComponentValue(tupleIndex, m_ComponentIndex), comparisonValue) != m_Invert); + } + return; } + + if(m_UnionType == IArrayThreshold::UnionOperator::And) + { + for(usize tupleIndex = 0; tupleIndex < numTuples; ++tupleIndex) + { + m_Output.setValue(tupleIndex, m_Output.getValue(tupleIndex) && (CompT{}(inputStore.getComponentValue(tupleIndex, m_ComponentIndex), comparisonValue) != m_Invert)); + } + return; + } + + if(m_UnionType == IArrayThreshold::UnionOperator::Or) + { + for(usize tupleIndex = 0; tupleIndex < numTuples; ++tupleIndex) + { + m_Output.setValue(tupleIndex, m_Output.getValue(tupleIndex) || (CompT{}(inputStore.getComponentValue(tupleIndex, m_ComponentIndex), comparisonValue) != m_Invert)); + } + return; + } + + throw std::runtime_error(fmt::format("Invalid threshold union operator: {}", static_cast(m_UnionType))); } template - void filterData(const AbstractDataStore& input, T trueValue, T falseValue) + void filterData(const AbstractDataStore& input) { if(m_ComparisonOperator == ArrayThreshold::ComparisonType::LessThan) { - filterDataWithComparision, T>(input, trueValue, falseValue); + filterDataWithComparision, T>(input); } else if(m_ComparisonOperator == ArrayThreshold::ComparisonType::GreaterThan) { - filterDataWithComparision, T>(input, trueValue, falseValue); + filterDataWithComparision, T>(input); } else if(m_ComparisonOperator == ArrayThreshold::ComparisonType::Operator_Equal) { - filterDataWithComparision, T>(input, trueValue, falseValue); + filterDataWithComparision, T>(input); } else if(m_ComparisonOperator == ArrayThreshold::ComparisonType::Operator_NotEqual) { - filterDataWithComparision, T>(input, trueValue, falseValue); + filterDataWithComparision, T>(input); } else { - std::string errorMessage = fmt::format("MultiThresholdObjects Comparison Operator not understood: '{}'", static_cast(m_ComparisonOperator)); + std::string errorMessage = fmt::format("MultiThresholdObjects Comparison Operator not understood: '{}'", static_cast(m_ComparisonOperator)); throw std::runtime_error(errorMessage); } } @@ -66,57 +153,24 @@ class ThresholdFilterHelper ArrayThreshold::ComparisonType m_ComparisonOperator; ArrayThreshold::ComparisonValue m_ComparisonValue; usize m_ComponentIndex = 0; - std::vector& m_Output; + IArrayThreshold::UnionOperator m_UnionType; + AbstractDataStore& m_Output; + bool m_Invert; + bool m_ReplaceOutput; }; struct ExecuteThresholdHelper { - template - void operator()(ThresholdFilterHelper& helper, const IDataArray& iDataArray, Type trueValue, Type falseValue) + template + void operator()(ThresholdFilterHelper& helper, const IDataArray& iDataArray) { const auto& dataStore = iDataArray.template getIDataStoreRefAs>(); - helper.template filterData(dataStore, trueValue, falseValue); + helper.template filterData(dataStore); } }; -/** - * @brief InsertThreshold - * @param numItems - * @param currentArrayPtr - * @param unionOperator - * @param newArrayPtr - * @param inverse - */ -template -void InsertThreshold(usize numItems, AbstractDataStore& currentStore, nx::core::IArrayThreshold::UnionOperator unionOperator, std::vector& newArrayPtr, bool inverse, T trueValue, T falseValue) +void ThresholdValue(const ArrayThreshold& comparisonValue, const DataStructure& dataStructure, AbstractDataStore& outputResultVector, bool replaceInput) { - for(usize i = 0; i < numItems; i++) - { - // invert the current comparison if necessary - if(inverse) - { - newArrayPtr[i] = (newArrayPtr[i] == trueValue) ? falseValue : trueValue; - } - - if(nx::core::IArrayThreshold::UnionOperator::Or == unionOperator) - { - currentStore[i] = (currentStore[i] == trueValue || newArrayPtr[i] == trueValue) ? trueValue : falseValue; - } - else if(currentStore[i] == falseValue || newArrayPtr[i] == falseValue) - { - currentStore[i] = falseValue; - } - } -} - -template -void ThresholdValue(const ArrayThreshold& comparisonValue, const DataStructure& dataStructure, AbstractDataStore& outputResultStore, int32_t& err, bool replaceInput, bool inverse, T trueValue, - T falseValue) -{ - // Get the total number of tuples, create and initialize an array with FALSE to use for these results - size_t totalTuples = outputResultStore.getNumberOfTuples(); - std::vector tempResultVector(totalTuples, falseValue); - nx::core::ArrayThreshold::ComparisonType compOperator = comparisonValue.getComparisonType(); nx::core::ArrayThreshold::ComparisonValue compValue = comparisonValue.getComparisonValue(); nx::core::IArrayThreshold::UnionOperator unionOperator = comparisonValue.getUnionOperator(); @@ -125,96 +179,100 @@ void ThresholdValue(const ArrayThreshold& comparisonValue, const DataStructure& usize componentIndex = comparisonValue.getComponentIndex(); - ThresholdFilterHelper helper(compOperator, compValue, componentIndex, tempResultVector); + // The first ThresholdValue in a set overwrites the accumulator rather than combining with it. + ThresholdFilterHelper helper(compOperator, compValue, componentIndex, unionOperator, outputResultVector, comparisonValue.isInverted(), replaceInput); const auto& iDataArray = dataStructure.getDataRefAs(inputDataArrayPath); - ExecuteDataFunction(ExecuteThresholdHelper{}, iDataArray.getDataType(), helper, iDataArray, trueValue, falseValue); - - if(replaceInput) - { - if(inverse) - { - std::reverse(tempResultVector.begin(), tempResultVector.end()); - } - // copy the temp uint8 vector to the final uint8 result array - for(size_t i = 0; i < totalTuples; i++) - { - outputResultStore[i] = tempResultVector[i]; - } - } - else - { - // insert into current threshold - InsertThreshold(totalTuples, outputResultStore, unionOperator, tempResultVector, inverse, trueValue, falseValue); - } + ExecuteDataFunction(ExecuteThresholdHelper{}, iDataArray.getDataType(), helper, iDataArray); } -struct ThresholdValueFunctor -{ - template - void operator()(const ArrayThreshold& comparisonValue, const DataStructure& dataStructure, IDataArray& outputResultArray, int32_t& err, bool replaceInput, bool inverse, T trueValue, T falseValue) - { - // Traditionally we would do a check to ensure we get a valid pointer, I'm forgoing that check because it - // was essentially done in the preflight part. - ThresholdValue(comparisonValue, dataStructure, outputResultArray.template getIDataStoreRefAs>(), err, replaceInput, inverse, trueValue, falseValue); - } -}; - -template -void ThresholdSet(const ArrayThresholdSet& inputComparisonSet, const DataStructure& dataStructure, AbstractDataStore& outputResultStore, int32_t& err, bool replaceInput, bool inverse, T trueValue, - T falseValue) +/** + * @brief Combines every child of a ThresholdSet into a single boolean result and returns it. + * + * The returned store holds the set's *inner* combination only; the set's own union operator and inversion + * flag are applied by whoever consumes the result, so that the top-level set and nested sets are handled + * identically. One store is allocated per nesting level and released as the recursion unwinds. + * + * @param inputComparisonSet Threshold set whose children should be combined. + * @param dataStructure DataStructure holding the input arrays. + * @param totalTuples Number of tuples in the mask being built. + * @param shouldCancel Cancel flag checked between thresholds. + * @return Store holding the combined result for this set. + */ +std::shared_ptr> ComputeThresholdSet(const ArrayThresholdSet& inputComparisonSet, const DataStructure& dataStructure, usize totalTuples, const std::atomic_bool& shouldCancel) { - // Get the total number of tuples, create and initialize an array with FALSE to use for these results - size_t totalTuples = outputResultStore.getNumberOfTuples(); - std::vector tempResultVector(totalTuples, falseValue); + auto resultStorePtr = DataStoreUtilities::CreateDataStore({totalTuples}, {1}, IDataAction::Mode::Execute); + AbstractDataStore& resultStore = *resultStorePtr.get(); + // The first child of the set writes every tuple of the accumulator rather than combining with it, so the + // store does not need to be pre-filled. The one case that leaves it untouched is a set with no children, + // handled after the loop; nothing here relies on the store being zero-initialized. bool firstValueFound = false; ArrayThresholdSet::CollectionType thresholds = inputComparisonSet.getArrayThresholds(); for(const std::shared_ptr& threshold : thresholds) { + if(shouldCancel) + { + return resultStorePtr; + } + const IArrayThreshold* thresholdPtr = threshold.get(); if(const auto* comparisonSet = dynamic_cast(thresholdPtr); comparisonSet != nullptr) { - ThresholdSet(*comparisonSet, dataStructure, outputResultStore, err, !firstValueFound, false, trueValue, falseValue); + auto childResultStorePtr = ComputeThresholdSet(*comparisonSet, dataStructure, totalTuples, shouldCancel); + if(shouldCancel) + { + return resultStorePtr; + } + ApplyThresholdValues(*comparisonSet, resultStore, *childResultStorePtr.get(), !firstValueFound); firstValueFound = true; } else if(const auto* comparisonValue = dynamic_cast(thresholdPtr); comparisonValue != nullptr) { - ThresholdValue(*comparisonValue, dataStructure, outputResultStore, err, !firstValueFound, false, trueValue, falseValue); + ThresholdValue(*comparisonValue, dataStructure, resultStore, !firstValueFound); firstValueFound = true; } } - if(replaceInput) - { - if(inverse) - { - std::reverse(tempResultVector.begin(), tempResultVector.end()); - } - // copy the temp uint8 vector to the final uint8 result array - for(size_t i = 0; i < totalTuples; i++) - { - outputResultStore[i] = tempResultVector[i]; - } - } - else + if(!firstValueFound) { - // insert into current threshold - InsertThreshold(totalTuples, outputResultStore, inputComparisonSet.getUnionOperator(), tempResultVector, inverse, trueValue, falseValue); + // A set with no children contributes nothing; define its result as all-false. + resultStore.fill(false); } + + return resultStorePtr; } struct ThresholdSetFunctor { template - void operator()(const ArrayThresholdSet& inputComparisonSet, const DataStructure& dataStructure, IDataArray& outputResultArray, int32_t& err, bool replaceInput, bool inverse, T trueValue, - T falseValue) + void operator()(const ArrayThresholdSet& inputComparisonSet, const DataStructure& dataStructure, IDataArray& outputResultArray, T trueValue, T falseValue, const std::atomic_bool& shouldCancel) { + if(shouldCancel) + { + return; + } + // Traditionally we would do a check to ensure we get a valid pointer, I'm forgoing that check because it // was essentially done in the preflight part. - ThresholdSet(inputComparisonSet, dataStructure, outputResultArray.template getIDataStoreRefAs>(), err, replaceInput, inverse, trueValue, falseValue); + auto& outputDataStore = outputResultArray.template getIDataStoreRefAs>(); + usize totalTuples = outputDataStore.getNumberOfTuples(); + auto resultStorePtr = ComputeThresholdSet(inputComparisonSet, dataStructure, totalTuples, shouldCancel); + if(shouldCancel) + { + return; + } + const AbstractDataStore& resultStore = *resultStorePtr.get(); + + // The top-level set's own inversion flag is applied here, mirroring what ApplyThresholdValues does for a + // nested set, while the boolean result is converted to the requested mask type. + const bool inverse = inputComparisonSet.isInverted(); + for(usize i = 0; i < totalTuples; i++) + { + outputDataStore.setValue(i, (resultStore.getValue(i) != inverse) ? trueValue : falseValue); + } } }; } // namespace @@ -246,30 +304,14 @@ Result<> MultiThresholdObjects::operator()() float64 trueValue = useCustomTrueValue ? customTrueValue : 1.0; float64 falseValue = useCustomFalseValue ? customFalseValue : 0.0; - bool firstValueFound = false; DataPath maskArrayPath = (*thresholdsObject.getRequiredPaths().begin()).replaceName(maskArrayName); - int32_t err = 0; - ArrayThresholdSet::CollectionType thresholdSet = thresholdsObject.getArrayThresholds(); - for(const std::shared_ptr& threshold : thresholdSet) + + if(m_ShouldCancel) { - if(m_ShouldCancel) - { - return {}; - } - const IArrayThreshold* thresholdPtr = threshold.get(); - if(const auto* comparisonSet = dynamic_cast(thresholdPtr); comparisonSet != nullptr) - { - ExecuteDataFunction(ThresholdSetFunctor{}, maskArrayType, *comparisonSet, m_DataStructure, m_DataStructure.getDataRefAs(maskArrayPath), err, !firstValueFound, - thresholdsObject.isInverted(), trueValue, falseValue); - firstValueFound = true; - } - else if(const auto* comparisonValue = dynamic_cast(thresholdPtr); comparisonValue != nullptr) - { - ExecuteDataFunction(ThresholdValueFunctor{}, maskArrayType, *comparisonValue, m_DataStructure, m_DataStructure.getDataRefAs(maskArrayPath), err, !firstValueFound, - thresholdsObject.isInverted(), trueValue, falseValue); - firstValueFound = true; - } + return {}; } + ExecuteDataFunction(ThresholdSetFunctor{}, maskArrayType, thresholdsObject, m_DataStructure, m_DataStructure.getDataRefAs(maskArrayPath), trueValue, falseValue, m_ShouldCancel); + return {}; } diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.cpp index 85c1535dc6..34dc6bff18 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.cpp @@ -164,7 +164,6 @@ IFilter::PreflightResult MultiThresholdObjectsFilter::preflightImpl(const DataSt // Check for same number of tuples and components usize numTuples = dataArray.getNumberOfTuples(); - usize numComponents = dataArray.getNumberOfComponents(); for(const auto& dataPath : thresholdPaths) { const auto& currentDataArray = dataStructure.getDataRefAs(dataPath); @@ -174,13 +173,6 @@ IFilter::PreflightResult MultiThresholdObjectsFilter::preflightImpl(const DataSt auto errorMessage = fmt::format("Data Arrays do not have same equal number of tuples. '{}:{}' and '{}:{}'", firstDataPath.toString(), numTuples, dataPath.toString(), currentNumTuples); return MakePreflightErrorResult(to_underlying(ErrorCodes::UnequalTuples), errorMessage); } - usize currentNumComponents = currentDataArray.getNumberOfComponents(); - if(currentNumComponents != numComponents) - { - auto errorMessage = - fmt::format("Data Arrays do not have same equal number of components. '{}:{}' and '{}:{}'", firstDataPath.toString(), numComponents, dataPath.toString(), currentNumComponents); - return MakePreflightErrorResult(to_underlying(ErrorCodes::UnequalComponents), errorMessage); - } } Result<> componentIndicesResult = CheckComponentIndicesInThresholds(thresholdsObject, dataStructure); @@ -223,8 +215,8 @@ IFilter::PreflightResult MultiThresholdObjectsFilter::preflightImpl(const DataSt } // Create the output boolean array - auto action = - std::make_unique(maskArrayType, dataArray.getIDataStoreRef().getTupleShape(), std::vector{1}, firstDataPath.replaceName(maskArrayName), dataArray.getDataFormat()); + const auto& dataStore = dataArray.getIDataStoreRef(); + auto action = std::make_unique(maskArrayType, dataStore.getTupleShape(), std::vector{1}, firstDataPath.replaceName(maskArrayName), dataArray.getDataFormat()); OutputActions actions; actions.appendAction(std::move(action)); diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.hpp index ed99745f3d..9ae1d6b50a 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.hpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.hpp @@ -37,7 +37,6 @@ class SIMPLNXCORE_EXPORT MultiThresholdObjectsFilter : public IFilter enum ErrorCodes : int64 { - UnequalComponents = -4001, UnequalTuples = -4002, CustomTrueWithBoolean = -4003, CustomFalseWithBoolean = -4004, diff --git a/src/Plugins/SimplnxCore/test/MultiThresholdObjectsTest.cpp b/src/Plugins/SimplnxCore/test/MultiThresholdObjectsTest.cpp index da9ded6ad8..a64dfb7922 100644 --- a/src/Plugins/SimplnxCore/test/MultiThresholdObjectsTest.cpp +++ b/src/Plugins/SimplnxCore/test/MultiThresholdObjectsTest.cpp @@ -28,53 +28,334 @@ const DataPath k_TestArrayIntPath = k_ImageCellDataName.createChildPath(k_TestAr const DataPath k_MultiComponentArrayPath = k_ImageCellDataName.createChildPath(k_MultiComponentArrayName); const DataPath k_ThresholdArrayPath = k_ImageCellDataName.createChildPath(k_ThresholdArrayName); -const DataPath k_MismatchingComponentsArrayPath = k_ImageCellDataName.createChildPath("MismatchingComponentsArray"); const DataPath k_MismatchingTuplesArrayPath({"MismatchingTuplesArray"}); +constexpr int8 k_TupleCount = 8; +constexpr int8 k_MultiComponentCount = 3; + +constexpr float64 k_FloatValueIncrement = 0.01; + +// The "Valid Execution, Mask DataType" test thresholds the float fixture (InputFloatValue(i) == (i + 1) * +// k_FloatValueIncrement) with GreaterThan. Tuple i is true exactly when i >= k_MaskTypeFirstTrueTuple, so the +// comparison value is derived from the split rather than the other way around. +constexpr usize k_MaskTypeFirstTrueTuple = 5; +constexpr float64 k_MaskTypeComparisonValue = k_MaskTypeFirstTrueTuple * k_FloatValueIncrement; +static_assert(k_MaskTypeFirstTrueTuple > 0 && k_MaskTypeFirstTrueTuple < static_cast(k_TupleCount), + "The comparison value must split the fixture so both the false and true branches are exercised"); + +constexpr std::array k_ExemplarInt4{0, 0, 0, 0, 0, 1, 1, 1}; +constexpr std::array k_ExemplarFloat02{0, 1, 0, 0, 0, 0, 0, 0}; + +constexpr int32 InputIntValue(int32 index) +{ + return index; +} + +constexpr int32 InputIntComponentValue(int32 tuple, int32 component) +{ + return (tuple + component) % 2 == 0 ? -tuple : tuple; +} + +constexpr float64 InputFloatValue(int32 index) +{ + return (index + 1) * k_FloatValueIncrement; +} + DataStructure CreateTestDataStructure() { DataStructure dataStructure; // Create two test arrays, a float array and a int array - // Set up geometry for tuples, a cuboid with dimensions 20, 10, 1 + // Set up geometry for tuples, a cuboid with dimensions k_TupleCount, 1, 1 ImageGeom* image = ImageGeom::Create(dataStructure, k_ImageGeometry); - std::vector dims = {20, 1, 1}; + std::vector dims = {k_TupleCount, 1, 1}; image->setDimensions(dims); - ShapeType tDims = {20}; + ShapeType tDims = {k_TupleCount}; ShapeType cDims = {1}; - ShapeType cDimsMulti = {3}; - float fnum = 0.0f; - int inum = 0; + ShapeType cDimsMulti = {k_MultiComponentCount}; + AttributeMatrix* am = AttributeMatrix::Create(dataStructure, k_CellData, tDims, image->getId()); Float32Array* data = Float32Array::CreateWithStore(dataStructure, k_TestArrayFloatName, tDims, cDims, am->getId()); Int32Array* data1 = Int32Array::CreateWithStore(dataStructure, k_TestArrayIntName, tDims, cDims, am->getId()); Int32Array* multiComponentData = Int32Array::CreateWithStore(dataStructure, k_MultiComponentArrayName, tDims, cDimsMulti, am->getId()); - Float32Array* invalid1 = Float32Array::CreateWithStore(dataStructure, k_MismatchingComponentsArrayPath.getTargetName(), tDims, cDimsMulti, am->getId()); - invalid1->fill(1.0); - Float32Array* invalid2 = Float32Array::CreateWithStore(dataStructure, k_MismatchingTuplesArrayPath.getTargetName(), std::vector{10}, cDims); - invalid2->fill(2.0); + // Mismatched tuple count throws an error. + // This is not true for mismatched component shapes. + Float32Array* problemArray = Float32Array::CreateWithStore(dataStructure, k_MismatchingTuplesArrayPath.getTargetName(), std::vector{10}, cDims); + problemArray->fill(2.0); usize numComponents = multiComponentData->getNumberOfComponents(); - int32 sign = 1; - - // Fill the float array with {.01,.02,.03,.04,.05,.06,.07,.08,.09,.10,.11,.12,.13,.14,.15.,16,.17,.18,.19,.20} - // Fill the int array with { 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19 } - // Fill multi-component array with {{0, 0, 0}, {1, -1, 1}, {-2, 2, -2}, ..., {17, -17, 17}, {-18, 18, -18}, {19, -19, 19}} - for(usize i = 0; i < 20; i++) - { - fnum += 0.01f; - (*data)[i] = fnum; // float array - (*data1)[i] = inum; // int array - multiComponentData->setComponent(i, 0, i * -sign); - multiComponentData->setComponent(i, 1, i * sign); - multiComponentData->setComponent(i, 2, i * -sign); - sign *= -1; - ++inum; + + // Fill the float array with {.01,.02,.03,.04,.05,.06,.07,.08} + // Fill the int array with {0,1,2,3,4,5,6,7} + // Fill multi-component array with {{0, 0, 0}, {1, -1, 1}, {-2, 2, -2}, {3, -3, 3}, {-4, 4, -4}, {5, -5, 5}, {-6, 6, -6}, {7, -7, 7}} + for(usize i = 0; i < k_TupleCount; i++) + { + (*data)[i] = InputFloatValue(i); // float array + (*data1)[i] = InputIntValue(i); // int array + + for(usize j = 0; j < k_MultiComponentCount; j++) + { + multiComponentData->setComponent(i, j, InputIntComponentValue(i, j)); + } } return dataStructure; } +template +void SetArrayValues(DataArray& dataArray) +{ + auto& dataStore = dataArray.getDataStoreRef(); + usize count = dataStore.size(); + for(usize i = 0; i < count; i++) + { + dataStore[i] = static_cast(i); + } +} + +DataStructure CreateTestDataStructure2() +{ + DataStructure dataStructure; + // Create two test arrays, a float array and a int array + // Set up geometry for tuples, a cuboid with dimensions k_TupleCount, 1, 1 + ImageGeom* image = ImageGeom::Create(dataStructure, k_ImageGeometry); + std::vector dims = {k_TupleCount, 1, 1}; + image->setDimensions(dims); + + ShapeType tDims = {k_TupleCount}; + ShapeType cDims = {1}; + ShapeType cDimsMulti = {k_MultiComponentCount}; + + AttributeMatrix* am = AttributeMatrix::Create(dataStructure, k_CellData, tDims, image->getId()); + auto* int8Array = Int8Array::CreateWithStore(dataStructure, "int8", tDims, cDims, am->getId()); + auto* int16Array = Int16Array::CreateWithStore(dataStructure, "int16", tDims, cDims, am->getId()); + auto* int32Array = Int32Array::CreateWithStore(dataStructure, "int32", tDims, cDims, am->getId()); + auto* int64Array = Int64Array::CreateWithStore(dataStructure, "int64", tDims, cDims, am->getId()); + auto* uint8Array = UInt8Array::CreateWithStore(dataStructure, "uint8", tDims, cDims, am->getId()); + auto* uint16Array = UInt16Array::CreateWithStore(dataStructure, "uint16", tDims, cDims, am->getId()); + auto* uint32Array = UInt32Array::CreateWithStore(dataStructure, "uint32", tDims, cDims, am->getId()); + auto* uint64Array = UInt64Array::CreateWithStore(dataStructure, "uint64", tDims, cDims, am->getId()); + auto* float32Array = Float32Array::CreateWithStore(dataStructure, "float32", tDims, cDims, am->getId()); + auto* float64Array = Float64Array::CreateWithStore(dataStructure, "float64", tDims, cDims, am->getId()); + auto* boolArray = BoolArray::CreateWithStore(dataStructure, "bool", tDims, cDims, am->getId()); + + SetArrayValues(*int8Array); + SetArrayValues(*int16Array); + SetArrayValues(*int32Array); + SetArrayValues(*int64Array); + SetArrayValues(*uint8Array); + SetArrayValues(*uint16Array); + SetArrayValues(*uint32Array); + SetArrayValues(*uint64Array); + SetArrayValues(*float32Array); + SetArrayValues(*float64Array); + SetArrayValues(*boolArray); + + return dataStructure; +} + +/** + * @brief Creates a single threshold for the filter to use. + * @param arrayPath Input DataArray path + * @param comparisonType type of comparison + * @param value Value to threshold against + * @param isInverted Should the threshold output be inverted + * componentIndex Component index of the array to threshold against. + */ +ArrayThresholdSet CreateSingleThreshold(const DataPath& arrayPath, ArrayThreshold::ComparisonType comparisonType, double value, bool isInverted, int componentIndex) +{ + ArrayThresholdSet thresholdSet; + auto threshold = std::make_shared(); + threshold->setArrayPath(arrayPath); + threshold->setComparisonType(comparisonType); + threshold->setComparisonValue(value); + threshold->setComponentIndex(componentIndex); + threshold->setInverted(isInverted); + thresholdSet.setArrayThresholds({threshold}); + + return thresholdSet; +} + +/** + * @brief Runs the MultiThresholdObjects filter on the provided threshold set + * @param dataStructure + * @param thresholdSet ThresholdSet to use for the MultiThresholdObjectsFilter + */ +void RunThresholdSetTest(DataStructure& dataStructure, ArrayThresholdSet thresholdSet) +{ + MultiThresholdObjectsFilter filter; + Arguments args; + + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::boolean)); + + // 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) + + // Require that the mask array only has one component + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + REQUIRE(thresholdArrayPtr->getNumberOfComponents() == 1); +} + +/** + * @brief Runs the MultiThresholdObjects filter on the provided DataStructure using a single array threshold. + * @param dataStructure + * @param arrayPath Path to use for the threshold DataArray + * @param comparisonType Type of comparison to perform + * @param value Value to threshold against + * @param isInverted should the output mask value be inverted + * @param componentIndex Which component of the array the threshold should use. + */ +void RunSingleThresholdTest(DataStructure& dataStructure, const DataPath& arrayPath, ArrayThreshold::ComparisonType comparisonType, double value, bool isInverted, int32 componentIndex = 0) +{ + auto thresholdSet = CreateSingleThreshold(arrayPath, comparisonType, value, isInverted, componentIndex); + RunThresholdSetTest(dataStructure, thresholdSet); +} + +// Integer checks +bool ExpectedIntSingleComponentMask(ArrayThreshold::ComparisonType comparisonType, int32 i, double thresholdValue, bool isInverted) +{ + bool expected = false; + + // The filter truncates the comparison value to the input array's type and compares in that type, matching + // legacy DREAM3D. A threshold of 5.5 against an int32 array therefore compares against 5. + const int32 comparisonValue = static_cast(thresholdValue); + + switch(comparisonType) + { + case ArrayThreshold::ComparisonType::GreaterThan: + expected = InputIntValue(i) > comparisonValue; + break; + case ArrayThreshold::ComparisonType::LessThan: + expected = InputIntValue(i) < comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_Equal: + expected = InputIntValue(i) == comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_NotEqual: + expected = InputIntValue(i) != comparisonValue; + break; + } + + if(isInverted) + { + expected = !expected; + } + return expected; +} + +void CheckIntTestDataSingleComponent(const DataStructure& dataStructure, ArrayThreshold::ComparisonType comparisonType, double thresholdValue, bool isInverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == ExpectedIntSingleComponentMask(comparisonType, i, thresholdValue, isInverted)); + } +} + +// Floating point checks +bool ExpectedFloatSingleComponentMask(ArrayThreshold::ComparisonType comparisonType, int32 i, double thresholdValue, bool isInverted) +{ + bool expected = false; + + // The target array is a Float32Array and the filter truncates the comparison value to the array's type + // before comparing, so the oracle rounds both operands through float32 the same way. Comparing the raw + // float64 values instead would diverge on exact-equality thresholds such as 0.03. + const float32 inputValue = static_cast(InputFloatValue(i)); + const float32 comparisonValue = static_cast(thresholdValue); + + switch(comparisonType) + { + case ArrayThreshold::ComparisonType::GreaterThan: + expected = inputValue > comparisonValue; + break; + case ArrayThreshold::ComparisonType::LessThan: + expected = inputValue < comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_Equal: + expected = inputValue == comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_NotEqual: + expected = inputValue != comparisonValue; + break; + } + + if(isInverted) + { + expected = !expected; + } + return expected; +} + +void CheckFloatTestDataSingleComponent(const DataStructure& dataStructure, ArrayThreshold::ComparisonType comparisonType, double thresholdValue, bool isInverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == ExpectedFloatSingleComponentMask(comparisonType, i, thresholdValue, isInverted)); + } +} + +// Multi-component checks +bool ExpectedIntMultiComponentMask(ArrayThreshold::ComparisonType comparisonType, int32 i, double thresholdValue, bool isInverted, int32 componentIndex) +{ + bool expected = false; + + // Same comparison-value truncation as the single-component int oracle above. + const int32 comparisonValue = static_cast(thresholdValue); + + switch(comparisonType) + { + case ArrayThreshold::ComparisonType::GreaterThan: + expected = InputIntComponentValue(i, componentIndex) > comparisonValue; + break; + case ArrayThreshold::ComparisonType::LessThan: + expected = InputIntComponentValue(i, componentIndex) < comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_Equal: + expected = InputIntComponentValue(i, componentIndex) == comparisonValue; + break; + case ArrayThreshold::ComparisonType::Operator_NotEqual: + expected = InputIntComponentValue(i, componentIndex) != comparisonValue; + break; + } + + if(isInverted) + { + expected = !expected; + } + return expected; +} + +void CheckIntTestDataMultiComponent(const DataStructure& dataStructure, ArrayThreshold::ComparisonType comparisonType, double thresholdValue, bool isInverted, int32 componentIndex) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == ExpectedIntMultiComponentMask(comparisonType, i, thresholdValue, isInverted, componentIndex)); + } +} + template float64 GetOutOfBoundsMinimumValue() { @@ -97,149 +378,526 @@ float64 GetOutOfBoundsMaximumValue() } } // namespace -TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution", "[SimplnxCore][MultiThresholdObjectsFilter]") +void CheckExemplar(const DataStructure& dataStructure, const std::array& exemplarMask) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == exemplarMask[i]); + } +} + +TEST_CASE("SimplnxCore::MultiThresholdObjects: Exemplar Single Thresholds: Int", "[SimplnxCore][MultiThresholdObjectsFilter]") { UnitTest::LoadPlugins(); DataStructure dataStructure = CreateTestDataStructure(); + const DataPath targetArray = k_TestArrayIntPath; + double thresholdValue = 4.0; + bool isInverted = false; - SECTION("Float Array Threshold") - { - MultiThresholdObjectsFilter filter; - Arguments args; + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted); + CheckExemplar(dataStructure, k_ExemplarInt4); + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); +TEST_CASE("SimplnxCore::MultiThresholdObjects: Exemplar Single Thresholds: Float", "[SimplnxCore][MultiThresholdObjectsFilter]") +{ + UnitTest::LoadPlugins(); - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::boolean)); + DataStructure dataStructure = CreateTestDataStructure(); + const DataPath targetArray = k_TestArrayFloatPath; + double thresholdValue = 0.02; + bool isInverted = false; - // Preflight the filter and check result - auto preflightResult = filter.preflight(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted); + CheckExemplar(dataStructure, k_ExemplarFloat02); + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} - // Execute the filter and check the result - auto executeResult = filter.execute(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Single Thresholds: Int", "[SimplnxCore][MultiThresholdObjectsFilter]") +{ + UnitTest::LoadPlugins(); - auto* thresholdArray = dataStructure.getDataAs(k_ThresholdArrayPath); - REQUIRE(thresholdArray != nullptr); + DataStructure dataStructure = CreateTestDataStructure(); + const DataPath targetArray = k_TestArrayIntPath; + double thresholdValue = GENERATE(-1.0, 0.0, 1.0, 2.0, 3.0, 4.0, 22.0, 5.5); + bool isInverted = GENERATE(false, true); - // For the comparison value of 0.1, the threshold array elements 0 to 9 should be false and 10 through 19 should be true - for(usize i = 0; i < 20; i++) - { - if(i < 10) - { - REQUIRE((*thresholdArray)[i] == false); - } - else - { - REQUIRE((*thresholdArray)[i] == true); - } - } + SECTION("ArrayThreshold: >") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted); + CheckIntTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted); } - SECTION("Int Array Threshold") + SECTION("ArrayThreshold: <") { - MultiThresholdObjectsFilter filter; - Arguments args; + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted); + CheckIntTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted); + } - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayIntPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(15); - thresholdSet.setArrayThresholds({threshold}); + SECTION("ArrayThreshold: ==") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted); + CheckIntTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted); + } + SECTION("ArrayThreshold: !=") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted); + CheckIntTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted); + } - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::boolean)); + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} - // Preflight the filter and check result - auto preflightResult = filter.preflight(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Single Thresholds: Float", "[SimplnxCore][MultiThresholdObjectsFilter]") +{ + UnitTest::LoadPlugins(); - // Execute the filter and check the result - auto executeResult = filter.execute(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) + DataStructure dataStructure = CreateTestDataStructure(); + const DataPath targetArray = k_TestArrayFloatPath; + double thresholdValue = GENERATE(0.0, 0.01, 0.02, 0.03, 0.04, 26.2); + bool isInverted = GENERATE(false, true); - auto* thresholdArray = dataStructure.getDataAs(k_ThresholdArrayPath); - REQUIRE(thresholdArray != nullptr); + SECTION("ArrayThreshold: >") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted); + CheckFloatTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted); + } - // For the comparison value of 0.1, the threshold array elements 0 to 9 should be false and 10 through 19 should be true - for(usize i = 0; i < 20; i++) - { - if(i <= 15) - { - REQUIRE((*thresholdArray)[i] == false); - } - else - { - REQUIRE((*thresholdArray)[i] == true); - } - } + SECTION("ArrayThreshold: <") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted); + CheckFloatTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted); + } + + SECTION("ArrayThreshold: ==") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted); + CheckFloatTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted); + } + SECTION("ArrayThreshold: !=") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted); + CheckFloatTestDataSingleComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted); } UnitTest::CheckArraysInheritTupleDims(dataStructure); } -TEMPLATE_TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution - Custom Values", "[SimplnxCore][MultiThresholdObjectsFilter]", int8, uint8, int16, uint16, int32, uint32, int64, uint64, - float32, float64) +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Single Thresholds: Int Multi-Component", "[SimplnxCore][MultiThresholdObjectsFilter]") { UnitTest::LoadPlugins(); - MultiThresholdObjectsFilter filter; DataStructure dataStructure = CreateTestDataStructure(); - Arguments args; + const DataPath targetArray = k_MultiComponentArrayPath; + double thresholdValue = GENERATE(-1.0, 0.0, 1.0, 2.0, 3.0, 4.0, 22.0, 5.5); + bool isInverted = GENERATE(false, true); + int32 componentIndex = GENERATE(0, 1, 2); - float64 trueValue = 25; - float64 falseValue = 10; + SECTION("ArrayThreshold: >") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted, componentIndex); + CheckIntTestDataMultiComponent(dataStructure, ArrayThreshold::ComparisonType::GreaterThan, thresholdValue, isInverted, componentIndex); + } - ArrayThresholdSet thresholdSet; + SECTION("ArrayThreshold: <") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted, componentIndex); + CheckIntTestDataMultiComponent(dataStructure, ArrayThreshold::ComparisonType::LessThan, thresholdValue, isInverted, componentIndex); + } + + SECTION("ArrayThreshold: ==") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted, componentIndex); + CheckIntTestDataMultiComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_Equal, thresholdValue, isInverted, componentIndex); + } + SECTION("ArrayThreshold: !=") + { + RunSingleThresholdTest(dataStructure, targetArray, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted, componentIndex); + CheckIntTestDataMultiComponent(dataStructure, ArrayThreshold::ComparisonType::Operator_NotEqual, thresholdValue, isInverted, componentIndex); + } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); +} + +/** + * @brief Creates a single threshold for the filter to use. + * @param arrayPath Input DataArray path + * @param comparisonType type of comparison + * @param value Value to threshold against + * @param isInverted Should the threshold output be inverted + * componentIndex Component index of the array to threshold against. + * unionOperator Union operator to apply on the threshold. Defaults to And + */ +std::shared_ptr CreateArrayThreshold(const DataPath& arrayPath, ArrayThreshold::ComparisonType comparisonType, double value, bool isInverted, int componentIndex, + ArrayThreshold::UnionOperator unionOperator = ArrayThreshold::UnionOperator::And) +{ auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayIntPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(15); - thresholdSet.setArrayThresholds({threshold}); + threshold->setArrayPath(arrayPath); + threshold->setComparisonType(comparisonType); + threshold->setComparisonValue(value); + threshold->setComponentIndex(componentIndex); + threshold->setInverted(isInverted); + threshold->setUnionOperator(unionOperator); + + return threshold; +} - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_UseCustomTrueValue, std::make_any(true)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CustomTrueValue, std::make_any(trueValue)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_UseCustomFalseValue, std::make_any(true)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CustomFalseValue, std::make_any(falseValue)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(GetDataType())); +ArrayThresholdSet CreateThresholdSet1() +{ + ArrayThresholdSet thresholdSet; - // Preflight the filter and check result - auto preflightResult = filter.preflight(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions) + // Threshold: Int > 2 + auto threshold1 = CreateArrayThreshold(k_TestArrayIntPath, ArrayThreshold::ComparisonType::GreaterThan, 2.0, false, 0, ArrayThreshold::UnionOperator::And); + // Threshold: Float < 0.025 + auto threshold2 = CreateArrayThreshold(k_TestArrayFloatPath, ArrayThreshold::ComparisonType::LessThan, 0.025, false, 0, ArrayThreshold::UnionOperator::And); + // Threshold: Int[1] > 0.0 : inverted + auto threshold3 = CreateArrayThreshold(k_MultiComponentArrayPath, ArrayThreshold::ComparisonType::GreaterThan, 0.0, true, 1, ArrayThreshold::UnionOperator::And); - // Execute the filter and check the result - auto executeResult = filter.execute(dataStructure, args); - SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) + thresholdSet.setArrayThresholds({threshold1, threshold2, threshold3}); - auto* thresholdArray = dataStructure.getDataAs>(k_ThresholdArrayPath); - REQUIRE(thresholdArray != nullptr); + return thresholdSet; +} - // For the comparison value of 0.1, the threshold array elements 0 to 9 should be false and 10 through 19 should be true - for(usize i = 0; i < 20; i++) +bool ExpectedThresholdSet1Mask(usize index, bool inverted) +{ + bool expectedThreshold1 = ExpectedIntSingleComponentMask(ArrayThreshold::ComparisonType::GreaterThan, index, 2.0, false); + bool expectedThreshold2 = ExpectedFloatSingleComponentMask(ArrayThreshold::ComparisonType::LessThan, index, 0.025, false); + bool expectedThreshold3 = ExpectedIntMultiComponentMask(ArrayThreshold::ComparisonType::GreaterThan, index, 0.0, true, 1); + + bool expected = expectedThreshold1 && expectedThreshold2 && expectedThreshold3; + if(inverted) + { + expected = !expected; + } + return expected; +} + +ArrayThresholdSet CreateThresholdSet2() +{ + ArrayThresholdSet thresholdSet; + + // Threshold: Int == 1 + auto threshold1 = CreateArrayThreshold(k_TestArrayIntPath, ArrayThreshold::ComparisonType::Operator_Equal, 1.0, false, 0, ArrayThreshold::UnionOperator::And); + // Threshold: Float != 5.0 + auto threshold2 = CreateArrayThreshold(k_TestArrayFloatPath, ArrayThreshold::ComparisonType::Operator_NotEqual, 5.0, false, 0, ArrayThreshold::UnionOperator::Or); + // Threshold: Int[0] < 0.0 : inverted + auto threshold3 = CreateArrayThreshold(k_MultiComponentArrayPath, ArrayThreshold::ComparisonType::LessThan, 0.0, true, 0, ArrayThreshold::UnionOperator::And); + + thresholdSet.setArrayThresholds({threshold1, threshold2, threshold3}); + + return thresholdSet; +} + +bool ExpectedThresholdSet2Mask(usize index, bool inverted) +{ + bool expectedThreshold1 = ExpectedIntSingleComponentMask(ArrayThreshold::ComparisonType::Operator_Equal, index, 1.0, false); + bool expectedThreshold2 = ExpectedFloatSingleComponentMask(ArrayThreshold::ComparisonType::Operator_NotEqual, index, 5.0, false); + bool expectedThreshold3 = ExpectedIntMultiComponentMask(ArrayThreshold::ComparisonType::LessThan, index, 0.0, true, 0); + + bool expected = (expectedThreshold1 || expectedThreshold2) && expectedThreshold3; + if(inverted) { - if(i <= 15) + expected = !expected; + } + return expected; +} + +ArrayThresholdSet CreateThresholdSet3() +{ + ArrayThresholdSet thresholdSet; + + auto set1 = std::make_shared(CreateThresholdSet1()); + auto set2 = std::make_shared(CreateThresholdSet2()); + + thresholdSet.setArrayThresholds({set1, set2}); + + return thresholdSet; +} + +ArrayThresholdSet CreateThresholdSet4() +{ + ArrayThresholdSet thresholdSet; + + auto set1 = std::make_shared(CreateThresholdSet1()); + auto set2 = std::make_shared(CreateThresholdSet2()); + set2->setUnionOperator(ArrayThreshold::UnionOperator::Or); + + thresholdSet.setArrayThresholds({set1, set2}); + + return thresholdSet; +} + +ArrayThresholdSet CreateThresholdSet5() +{ + ArrayThresholdSet thresholdSet; + + auto set1 = std::make_shared(CreateThresholdSet1()); + auto set2 = std::make_shared(CreateThresholdSet2()); + set2->setUnionOperator(ArrayThreshold::UnionOperator::Or); + set2->setInverted(true); + + thresholdSet.setArrayThresholds({set1, set2}); + + return thresholdSet; +} + +/** + * @brief Creates a ThresholdSet whose children mix a leaf ArrayThreshold with a sibling nested ArrayThresholdSet. + * + * This is the shape that triggered MultiThresholdObjectsFilter-D1, where the mask came back all-false + * regardless of input. Every other CreateThresholdSet* helper passes either all leaves or all nested sets, so + * this shape had no in-repo coverage. + */ +ArrayThresholdSet CreateThresholdSet6() +{ + ArrayThresholdSet thresholdSet; + + // Threshold: Int > 2 + auto leafThreshold = CreateArrayThreshold(k_TestArrayIntPath, ArrayThreshold::ComparisonType::GreaterThan, 2.0, false, 0, ArrayThreshold::UnionOperator::And); + auto nestedSet = std::make_shared(CreateThresholdSet2()); + + thresholdSet.setArrayThresholds({leafThreshold, nestedSet}); + + return thresholdSet; +} + +/** + * @brief Creates a ThresholdSet mixing a leaf ArrayThreshold with a sibling *inverted* nested ArrayThresholdSet. + * + * This is the shape that triggered MultiThresholdObjectsFilter-D2, where inversion of a nested set reversed + * the tuple order instead of flipping each tuple's value. + */ +ArrayThresholdSet CreateThresholdSet7() +{ + ArrayThresholdSet thresholdSet; + + // Threshold: Int > 2 + auto leafThreshold = CreateArrayThreshold(k_TestArrayIntPath, ArrayThreshold::ComparisonType::GreaterThan, 2.0, false, 0, ArrayThreshold::UnionOperator::And); + auto nestedSet = std::make_shared(CreateThresholdSet2()); + nestedSet->setUnionOperator(ArrayThreshold::UnionOperator::Or); + nestedSet->setInverted(true); + + thresholdSet.setArrayThresholds({leafThreshold, nestedSet}); + + return thresholdSet; +} + +void CheckThresholdSet1(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == ExpectedThresholdSet1Mask(i, inverted)); + } +} + +void CheckThresholdSet2(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + REQUIRE(thresholdStore[i] == ExpectedThresholdSet2Mask(i, inverted)); + } +} + +void CheckThresholdSet3(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + bool expectedMask1 = ExpectedThresholdSet1Mask(i, false); + bool expectedMask2 = ExpectedThresholdSet2Mask(i, false); + + bool expected = expectedMask1 && expectedMask2; + if(inverted) { - REQUIRE((*thresholdArray)[i] == falseValue); + expected = !expected; } - else + + REQUIRE(thresholdStore[i] == expected); + } +} + +void CheckThresholdSet4(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + bool expectedMask1 = ExpectedThresholdSet1Mask(i, false); + bool expectedMask2 = ExpectedThresholdSet2Mask(i, false); + + bool expected = expectedMask1 || expectedMask2; + if(inverted) + { + expected = !expected; + } + + REQUIRE(thresholdStore[i] == expected); + } +} + +void CheckThresholdSet5(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + bool expectedMask1 = ExpectedThresholdSet1Mask(i, false); + bool expectedMask2 = ExpectedThresholdSet2Mask(i, true); + + bool expected = expectedMask1 || expectedMask2; + if(inverted) + { + expected = !expected; + } + + REQUIRE(thresholdStore[i] == expected); + } +} + +void CheckThresholdSet6(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + // The leaf is the first child, so it seeds the set's accumulator; the nested set then combines with its + // own And union operator. + bool expectedLeaf = ExpectedIntSingleComponentMask(ArrayThreshold::ComparisonType::GreaterThan, i, 2.0, false); + bool expectedNested = ExpectedThresholdSet2Mask(i, false); + + bool expected = expectedLeaf && expectedNested; + if(inverted) + { + expected = !expected; + } + + REQUIRE(thresholdStore[i] == expected); + } +} + +void CheckThresholdSet7(DataStructure& dataStructure, bool inverted) +{ + const auto* thresholdArrayPtr = dataStructure.getDataAs(k_ThresholdArrayPath); + REQUIRE(thresholdArrayPtr != nullptr); + + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); + + for(usize i = 0; i < k_TupleCount; i++) + { + bool expectedLeaf = ExpectedIntSingleComponentMask(ArrayThreshold::ComparisonType::GreaterThan, i, 2.0, false); + // The nested set is inverted, which must flip each tuple's value rather than reverse the tuple order. + bool expectedNested = ExpectedThresholdSet2Mask(i, true); + + bool expected = expectedLeaf || expectedNested; + if(inverted) { - REQUIRE((*thresholdArray)[i] == trueValue); + expected = !expected; } + + REQUIRE(thresholdStore[i] == expected); + } +} + +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Threshold Sets", "[SimplnxCore][MultiThresholdObjectsFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = CreateTestDataStructure(); + bool isInverted = GENERATE(false, true); + + SECTION("ArraySet 1") + { + auto thresholdSet = CreateThresholdSet1(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet1(dataStructure, isInverted); + } + + SECTION("ArraySet 2") + { + auto thresholdSet = CreateThresholdSet2(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet2(dataStructure, isInverted); + } + + SECTION("ArraySet 3") + { + auto thresholdSet = CreateThresholdSet3(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet3(dataStructure, isInverted); + } + + SECTION("ArraySet 4") + { + auto thresholdSet = CreateThresholdSet4(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet4(dataStructure, isInverted); + } + + SECTION("ArraySet 5") + { + auto thresholdSet = CreateThresholdSet5(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet5(dataStructure, isInverted); } + + // Regression coverage for MultiThresholdObjectsFilter-D1: a set mixing a leaf threshold with a sibling + // nested set produced an all-false mask regardless of input. + SECTION("ArraySet 6: leaf + nested set") + { + auto thresholdSet = CreateThresholdSet6(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet6(dataStructure, isInverted); + } + + // Regression coverage for MultiThresholdObjectsFilter-D2: an inverted nested set reversed the tuple order + // instead of flipping each tuple's value. + SECTION("ArraySet 7: leaf + inverted nested set") + { + auto thresholdSet = CreateThresholdSet7(); + thresholdSet.setInverted(isInverted); + RunThresholdSetTest(dataStructure, thresholdSet); + CheckThresholdSet7(dataStructure, isInverted); + } + + UnitTest::CheckArraysInheritTupleDims(dataStructure); } +// Invalid executions + TEST_CASE("SimplnxCore::MultiThresholdObjects: Invalid Execution", "[SimplnxCore][MultiThresholdObjectsFilter]") { UnitTest::LoadPlugins(); @@ -260,23 +918,8 @@ TEST_CASE("SimplnxCore::MultiThresholdObjects: Invalid Execution", "[SimplnxCore ArrayThresholdSet thresholdSet; auto threshold = std::make_shared(); threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - } - SECTION("Mismatching Components in Threshold Arrays") - { - ArrayThresholdSet thresholdSet; - auto threshold1 = std::make_shared(); - threshold1->setArrayPath(k_TestArrayFloatPath); - threshold1->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold1->setComparisonValue(0.1); - auto threshold2 = std::make_shared(); - threshold2->setArrayPath(k_MismatchingComponentsArrayPath); - threshold2->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold2->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold1, threshold2}); + threshold->setComparisonValue(0.1); + thresholdSet.setArrayThresholds({threshold}); args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); } @@ -380,6 +1023,8 @@ TEMPLATE_TEST_CASE("SimplnxCore::MultiThresholdObjects: Invalid Execution - Out SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions); REQUIRE(preflightResult.outputActions.errors().size() == 1); REQUIRE(preflightResult.outputActions.errors()[0].code == code); + + UnitTest::CheckArraysInheritTupleDims(dataStructure); } TEST_CASE("SimplnxCore::MultiThresholdObjects: Invalid Execution - Boolean Custom Values", "[SimplnxCore][MultiThresholdObjectsFilter]") @@ -424,328 +1069,245 @@ TEST_CASE("SimplnxCore::MultiThresholdObjects: Invalid Execution - Boolean Custo UnitTest::CheckArraysInheritTupleDims(dataStructure); } +// DataType checks + template void checkMaskValues(const DataStructure& dataStructure, const DataPath& thresholdArrayPath) { auto* thresholdArrayPtr = dataStructure.getDataAs>(thresholdArrayPath); REQUIRE(thresholdArrayPtr != nullptr); - auto& thresholdArray = (*thresholdArrayPtr); + auto& thresholdStore = thresholdArrayPtr->getDataStoreRef(); - // For the comparison value of 0.1, the threshold array elements 0 to 9 should be false and 10 through 19 should be true - for(usize i = 0; i < 20; i++) + // Tuples below k_MaskTypeFirstTrueTuple are false and the rest are true. The split is taken from the + // fixture constants so that changing k_TupleCount cannot silently make one of the two branches unreachable. + for(usize i = 0; i < k_TupleCount; i++) { - if(i < 10) + if(i < k_MaskTypeFirstTrueTuple) { - REQUIRE(thresholdArray[i] == static_cast(0)); + REQUIRE(thresholdStore[i] == static_cast(0)); } else { - REQUIRE(thresholdArray[i] == static_cast(1)); + REQUIRE(thresholdStore[i] == static_cast(1)); } } } -TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution, DataType", "[SimplnxCore][MultiThresholdObjectsFilter]") +template +void runMaskTypeFilter(MultiThresholdObjectsFilter& filter, Arguments& args, DataStructure& dataStructure) +{ + // 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) + + checkMaskValues(dataStructure, k_ThresholdArrayPath); +} + +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution, Mask DataType", "[SimplnxCore][MultiThresholdObjectsFilter]") { UnitTest::LoadPlugins(); DataStructure dataStructure = CreateTestDataStructure(); + // Shared filter setup + MultiThresholdObjectsFilter filter; + Arguments args; + + ArrayThresholdSet thresholdSet; + auto threshold = std::make_shared(); + threshold->setArrayPath(k_TestArrayFloatPath); + threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); + threshold->setComparisonValue(k_MaskTypeComparisonValue); + thresholdSet.setArrayThresholds({threshold}); + + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); + // Signed SECTION("Int8 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::int8)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("Int16 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::int16)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("Int32 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::int32)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("Int64 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::int64)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } // Unsigned SECTION("UInt8 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::uint8)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("UInt16 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::uint16)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("UInt32 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::uint32)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("UInt64 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::uint64)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } // Floating Point SECTION("Float32 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::float32)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } SECTION("Float64 Threshold") { - MultiThresholdObjectsFilter filter; - Arguments args; - - ArrayThresholdSet thresholdSet; - auto threshold = std::make_shared(); - threshold->setArrayPath(k_TestArrayFloatPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0.1); - thresholdSet.setArrayThresholds({threshold}); - - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::float64)); - // 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) - - checkMaskValues(dataStructure, k_ThresholdArrayPath); + runMaskTypeFilter(filter, args, dataStructure); } UnitTest::CheckArraysInheritTupleDims(dataStructure); } -TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution - Multicomponent", "[SimplnxCore][MultiThresholdObjectsFilter]") +void TestMaskOutputForInputType(Int8AbstractDataStore& mask, float64 comparisonValue) { - DataStructure dataStructure = CreateTestDataStructure(); + usize count = mask.size(); + for(usize i = 0; i < count; i++) + { + int8 targetValue = (i < comparisonValue) ? 1 : 0; + REQUIRE(static_cast(mask[i]) == targetValue); + } +} +void TestMaskOutputForBoolInputType(Int8AbstractDataStore& mask, float64 comparisonValue) +{ + usize count = mask.size(); + for(usize i = 0; i < count; i++) + { + int8 targetValue = (static_cast(i) < comparisonValue) ? 1 : 0; + REQUIRE(static_cast(mask[i]) == targetValue); + } +} + +TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution, Input Array DataType", "[SimplnxCore][MultiThresholdObjectsFilter]") +{ + UnitTest::LoadPlugins(); + + DataStructure dataStructure = CreateTestDataStructure2(); + float64 comparisonValue = 3.0; + DataPath matrixPath({k_ImageGeometry, k_CellData}); + bool isBoolInput = false; + + // Shared filter setup MultiThresholdObjectsFilter filter; Arguments args; ArrayThresholdSet thresholdSet; auto threshold = std::make_shared(); - threshold->setArrayPath(k_MultiComponentArrayPath); - threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); - threshold->setComparisonValue(0); - threshold->setComponentIndex(1); + threshold->setComparisonType(ArrayThreshold::ComparisonType::LessThan); + threshold->setComparisonValue(comparisonValue); thresholdSet.setArrayThresholds({threshold}); - args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + // Signed + SECTION("Int8") + { + threshold->setArrayPath(matrixPath.createChildPath("int8")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("Int16") + { + threshold->setArrayPath(matrixPath.createChildPath("int16")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("Int32") + { + threshold->setArrayPath(matrixPath.createChildPath("int32")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("Int64") + { + threshold->setArrayPath(matrixPath.createChildPath("int64")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + // Unsigned + SECTION("UInt8") + { + threshold->setArrayPath(matrixPath.createChildPath("uint8")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("UInt16") + { + threshold->setArrayPath(matrixPath.createChildPath("uint16")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("UInt32") + { + threshold->setArrayPath(matrixPath.createChildPath("uint32")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("UInt64") + { + threshold->setArrayPath(matrixPath.createChildPath("uint64")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + // Floating Point + SECTION("Float32") + { + threshold->setArrayPath(matrixPath.createChildPath("float32")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + SECTION("Float64") + { + threshold->setArrayPath(matrixPath.createChildPath("float64")); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + } + // Bool + SECTION("Boolean") + { + threshold->setArrayPath(matrixPath.createChildPath("bool")); + comparisonValue = 0.9; + threshold->setComparisonValue(comparisonValue); + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + isBoolInput = true; + } + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); - args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::boolean)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(DataType::int8)); // Preflight the filter and check result auto preflightResult = filter.preflight(dataStructure, args); @@ -755,31 +1317,81 @@ TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution - Multicomponent" auto executeResult = filter.execute(dataStructure, args); SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result) - auto* thresholdArray = dataStructure.getDataAs(k_ThresholdArrayPath); - REQUIRE(thresholdArray != nullptr); - - usize numTuples = thresholdArray->getNumberOfTuples(); - - // (x, y, z) - // y > 0 - // even tuple indices should be true except 0 - REQUIRE_FALSE((*thresholdArray)[0]); - for(usize i = 1; i < numTuples; i++) + auto* maskArray = dataStructure.getDataAs(matrixPath.createChildPath(k_ThresholdArrayName)); + REQUIRE(maskArray != nullptr); + auto& maskStore = maskArray->getDataStoreRef(); + // Bool input + if(isBoolInput) { - bool value = (*thresholdArray)[i]; - if(i % 2 == 0) - { - REQUIRE(value); - } - else - { - REQUIRE_FALSE(value); - } + TestMaskOutputForBoolInputType(maskStore, comparisonValue); + } + else + { + TestMaskOutputForInputType(maskStore, comparisonValue); } UnitTest::CheckArraysInheritTupleDims(dataStructure); } +TEST_CASE("SimplnxCore::MultiThresholdObjectsFilter: SIMPL Nested Set Conversion", "[SimplnxCore][MultiThresholdObjectsFilter][BackwardsCompatibility]") +{ + auto app = Application::GetOrCreateInstance(); + UnitTest::LoadPlugins(); + auto filterList = app->getFilterList(); + + // A legacy "Threshold Objects (Advanced)" pipeline whose thresholds are a leaf sibling to an inverted + // nested set, with the nested set's children joined by Or: + // + // Int32 > 20 AND NOT( Float32 < 0.8 OR Int32 == 55 ) + // + // The conversion used to flatten the nested set into the parent list and drop every union operator, which + // silently turned this into a four-way AND over the leaves. Both the nesting and the Or must survive. + const fs::path fixturePath = fs::path(nx::core::unit_test::k_SourceDir.view()) / "test" / "simpl_conversion" / "6_5" / "MultiThresholdObjectsFilter_Nested.json"; + + 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 Arguments args = pipelineFilter->getArguments(); + auto thresholds = args.value(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key); + + auto topLevel = thresholds.getArrayThresholds(); + REQUIRE(topLevel.size() == 2); + + // Child 0: the leaf. SIMPL encodes GreaterThan as 1 and SIMPLNX as 0, so the operator must be swapped. + auto leaf = std::dynamic_pointer_cast(topLevel[0]); + REQUIRE(leaf != nullptr); + CHECK(leaf->getComparisonType() == ArrayThreshold::ComparisonType::GreaterThan); + CHECK(leaf->getComparisonValue() == 20.0); + CHECK(leaf->getArrayPath() == DataPath({"DataContainer", "CellData", "Int32"})); + + // Child 1: the nested set must still be a set, still inverted, and still hold both of its own children. + auto nestedSet = std::dynamic_pointer_cast(topLevel[1]); + REQUIRE(nestedSet != nullptr); + CHECK(nestedSet->isInverted()); + CHECK(nestedSet->getUnionOperator() == IArrayThreshold::UnionOperator::And); + + auto nestedChildren = nestedSet->getArrayThresholds(); + REQUIRE(nestedChildren.size() == 2); + + auto nestedFirst = std::dynamic_pointer_cast(nestedChildren[0]); + REQUIRE(nestedFirst != nullptr); + CHECK(nestedFirst->getComparisonType() == ArrayThreshold::ComparisonType::LessThan); + CHECK(nestedFirst->getComparisonValue() == 0.8); + + // The Or joining the nested set's children is the part the old flattening pass discarded. + auto nestedSecond = std::dynamic_pointer_cast(nestedChildren[1]); + REQUIRE(nestedSecond != nullptr); + CHECK(nestedSecond->getComparisonType() == ArrayThreshold::ComparisonType::Operator_Equal); + CHECK(nestedSecond->getComparisonValue() == 55.0); + CHECK(nestedSecond->getUnionOperator() == IArrayThreshold::UnionOperator::Or); +} + TEST_CASE("SimplnxCore::MultiThresholdObjectsFilter: SIMPL Backwards Compatibility", "[SimplnxCore][MultiThresholdObjectsFilter][BackwardsCompatibility]") { auto app = Application::GetOrCreateInstance(); @@ -815,3 +1427,59 @@ TEST_CASE("SimplnxCore::MultiThresholdObjectsFilter: SIMPL Backwards Compatibili } } } + +TEMPLATE_TEST_CASE("SimplnxCore::MultiThresholdObjects: Valid Execution - Custom Values", "[SimplnxCore][MultiThresholdObjectsFilter]", int8, uint8, int16, uint16, int32, uint32, int64, uint64, + float32, float64) +{ + UnitTest::LoadPlugins(); + + MultiThresholdObjectsFilter filter; + DataStructure dataStructure = CreateTestDataStructure(); + Arguments args; + + float64 trueValue = 25; + float64 falseValue = 10; + const int32 comparisonValue = 3; + + ArrayThresholdSet thresholdSet; + auto threshold = std::make_shared(); + threshold->setArrayPath(k_TestArrayIntPath); + threshold->setComparisonType(ArrayThreshold::ComparisonType::GreaterThan); + threshold->setComparisonValue(comparisonValue); + thresholdSet.setArrayThresholds({threshold}); + + args.insertOrAssign(MultiThresholdObjectsFilter::k_ArrayThresholdsObject_Key, std::make_any(thresholdSet)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedDataName_Key, std::make_any(k_ThresholdArrayName)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_UseCustomTrueValue, std::make_any(true)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CustomTrueValue, std::make_any(trueValue)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_UseCustomFalseValue, std::make_any(true)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CustomFalseValue, std::make_any(falseValue)); + args.insertOrAssign(MultiThresholdObjectsFilter::k_CreatedMaskType_Key, std::make_any(GetDataType())); + + // 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) + + UnitTest::CheckArraysInheritTupleDims(dataStructure); + + auto* thresholdArray = dataStructure.getDataAs>(k_ThresholdArrayPath); + REQUIRE(thresholdArray != nullptr); + auto& thresholdStore = thresholdArray->getDataStoreRef(); + + // Use tuple count constant in case the underlying data size changes. + for(usize i = 0; i < k_TupleCount; i++) + { + if(i <= comparisonValue) + { + REQUIRE(thresholdStore[i] == falseValue); + } + else + { + REQUIRE(thresholdStore[i] == trueValue); + } + } +} diff --git a/src/Plugins/SimplnxCore/test/simpl_conversion/6_5/MultiThresholdObjectsFilter_Nested.json b/src/Plugins/SimplnxCore/test/simpl_conversion/6_5/MultiThresholdObjectsFilter_Nested.json new file mode 100644 index 0000000000..bbeec87752 --- /dev/null +++ b/src/Plugins/SimplnxCore/test/simpl_conversion/6_5/MultiThresholdObjectsFilter_Nested.json @@ -0,0 +1,44 @@ +{ + "PipelineBuilder": { + "Name": "Multi Threshold Objects nested-set conversion fixture", + "Number_Filters": 1, + "Version": 6 + }, + "0": { + "Filter_Enabled": true, + "Filter_Human_Label": "Multi Threshold Objects (Advanced)", + "Filter_Name": "MultiThresholdObjects2", + "Filter_Uuid": "{686d5393-2b02-5c86-b887-dd81a8ae80f2}", + "SelectedThresholds": { + "Data Container Name": "DataContainer", + "Attribute Matrix Name": "CellData", + "Thresholds": [ + { + "Union Operator": 0, + "Attribute Array Name": "Int32", + "Comparison Operator": 1, + "Comparison Value": 20.0 + }, + { + "Union Operator": 0, + "Invert Comparison": true, + "Comparison Values": [ + { + "Union Operator": 1, + "Attribute Array Name": "Float32", + "Comparison Operator": 0, + "Comparison Value": 0.8 + }, + { + "Union Operator": 1, + "Attribute Array Name": "Int32", + "Comparison Operator": 2, + "Comparison Value": 55.0 + } + ] + } + ] + }, + "DestinationArrayName": "TestName" + } +} diff --git a/src/Plugins/SimplnxCore/vv/MultiThresholdObjectsFilter.md b/src/Plugins/SimplnxCore/vv/MultiThresholdObjectsFilter.md new file mode 100644 index 0000000000..5304b99039 --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/MultiThresholdObjectsFilter.md @@ -0,0 +1,174 @@ +# V&V Report: MultiThresholdObjectsFilter + +| | | +|--------|--------------| +| Plugin | SimplnxCore | +| SIMPLNX UUID | `4246245e-1011-4add-8436-0af6bed19228` | +| DREAM3D 6.5.171 equivalent | Two separate legacy filters, consolidated: **Threshold Objects** (`MultiThresholdObjects`, SIMPL UUID `014b7300-cf36-5ede-a751-5faf9b119dae`) and **Threshold Objects (Advanced)** (`MultiThresholdObjects2`, SIMPL UUID `686d5393-2b02-5c86-b887-dd81a8ae80f2`) — both mapped to this single filter's UUID in `SimplnxCoreLegacyUUIDMapping.hpp` (see Algorithm Relationship) | +| Verified commit | ** | +| Status | COMPLETE — 2026-08-19 | +| Sign-off | Matthew Marine (V&V author, PR #1688). Second engineer: Michael A. Jackson , 2026-08-19 (PR #1688 review). | + +## At a glance + +| Aspect | Current state | +|------------------------|------------------------------------------------------------------------------------------------------------------------------| +| Algorithm Relationship | **Rewrite.** Consolidates two independently-shipped legacy filters — **Threshold Objects** (flat, AND-only) and **Threshold Objects (Advanced)** (nested AND/OR sets) — into one SIMPLNX filter under one new UUID, unified around a single `ArrayThresholdSet` model. Not a line-by-line translation of either legacy source. | +| Oracle (confirmed) | **Class 1 (Analytical) — confirmed.** `expected[i] = COMPARISON(input[i], value)`, hand-combined via AND/OR/invert boolean algebra. Encoded across 14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations in `MultiThresholdObjectsTest.cpp`, all pass. | +| Code paths enumerated | **26 of 27 exercised.** Row 13 (unreachable comparison-operator `else`-throw) is a permanent, acceptable gap and the only one remaining. Row 25 (a set mixing a leaf threshold with a nested set — the `MultiThresholdObjectsFilter-D1` trigger shape) is now covered by the `ArraySet 6` / `ArraySet 7` `SECTION`s of `Valid Threshold Sets`. | +| Tests today | **14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations / 31 ctest entries**, all passing in both the in-core and out-of-core Release builds (12 single-entry TEST_CASEs + 2 `TEMPLATE_TEST_CASE`s instantiated over 9 and 10 types respectively). Exhaustive sweeps over comparison operator × invert × union operator × set nesting × mask `DataType` (10 types, plus boolean covered elsewhere) × source-array `DataType` (11 types) × custom TRUE/FALSE execution (10 types), plus negative/error-path groups and a SIMPL backwards-compatibility check. All fixtures built in-memory. | +| Exemplar archive | **None.** All fixtures are constructed in-memory by `CreateTestDataStructure()` / `CreateTestDataStructure2()`; no `.dream3d` exemplar or `download_test_data()` entry exists for this filter. | +| Legacy comparison | **Run, and now complete — no configurations outstanding.** Independent three-way A/B (DREAM3D 6.5.171 `PipelineRunner` vs. this branch's `nxrunner` vs. an independent numpy oracle) across six configurations: flat/basic (`AB1`, vs. `MultiThresholdObjects`), nested and inverted-nested (`AB2`/`AB3`), and three deeper shapes added at second-engineer review — three-level nesting, an inverted sibling set, and a grouping-sensitive case (`AB4`–`AB6`, vs. `MultiThresholdObjects2`) — plus a 50M-tuple scale re-run of `AB1`–`AB3`. All sources MATCH in every case. Pre-fix (`develop`): `AB2`/`AB3` diverged (38/100 and 51/100 tuples wrong) — see `-D1`/`-D2`. `AB4`–`AB6` were additionally run through SIMPLNX's SIMPL-pipeline conversion, which surfaced `-D4`. | +| Bug flags | **Three, all fixed by this PR, all quantified against real legacy output, all with in-repo regression coverage.** `-D1` — a set combining a leaf threshold with a sibling nested set produced an all-false mask (38/100 tuples wrong) on `develop`. `-D2` — an inverted nested set used `std::reverse` to flip tuple *order* instead of each tuple's value (51/100 wrong) on `develop`. `-D4` — **found at second-engineer review**: converting a legacy pipeline with a nested threshold set flattened the nesting and reset every union operator to AND, so converted `Threshold Objects (Advanced)` pipelines computed a plain AND over all leaves (55/100, 50/100 and 4/100 wrong on `AB4`–`AB6`); pre-existing on `develop`, in shared `ArrayThresholdsParameter` conversion code. See `vv/deviations/MultiThresholdObjectsFilter.md`. | +| V&V phase | Discovery, algorithm relationship, oracle design + reconciliation, algorithm review (fixes applied and re-verified), test inventory, legacy comparison, deviations — **complete**. Oracle chosen and applied (Class 1, corroborated by an independent numpy oracle in the legacy A/B), code paths enumerated (26/27), legacy A/B run and MATCH at both 100-tuple and 50M-tuple scale, 6 deviations documented (`D1`/`D2`/`D4` bugs fixed by this PR, each with an in-repo regression test; `D3`/`D5`/`D6` confirmed non-bug capability differences). Second-engineer review of the oracle design and this report **signed off by Michael A. Jackson, 2026-08-19** (PR #1688), which also closed the last three outstanding legacy-comparison items and found `D4`. **Nothing outstanding.** | + +## Summary + +`MultiThresholdObjectsFilter` builds a typed mask array by elementwise-comparing one or more input arrays against user-supplied thresholds, combined through an arbitrarily-nested tree of AND/OR/invert `ArrayThresholdSet`s. Verification uses a **Class 1 (Analytical) oracle**: every comparison operator, invert flag, union operator, nesting depth, custom TRUE/FALSE execution, and both the mask-output and source-input `DataType` are exhaustively hand-derived and asserted in `MultiThresholdObjectsTest.cpp` (14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations, all passing). 26 of 27 algorithm, preflight and SIMPL-conversion code paths are exercised. An independent three-way runtime A/B (legacy DREAM3D 6.5.171, this branch, and a numpy oracle) against both legacy predecessors — at 100 tuples and again at 50M tuples — confirms the current implementation matches legacy exactly, and quantifies two real bugs that were present on `develop` and are fixed by this PR: `MultiThresholdObjectsFilter-D1` (all-false mask when a set mixes a leaf threshold with a nested set, 38/100 tuples wrong) and `MultiThresholdObjectsFilter-D2` (`std::reverse`-based tuple-order corruption in an inverted nested set instead of per-value inversion, 51/100 tuples wrong). Both now have dedicated in-repo regression coverage (`Valid Threshold Sets` → `ArraySet 6` / `ArraySet 7`), each verified to fail when the algorithm is stubbed out. Second-engineer review extended the A/B to three deeper shapes (`AB4`–`AB6`) and to SIMPLNX's own SIMPL-pipeline conversion path, which surfaced a third bug, `MultiThresholdObjectsFilter-D4`: converting a legacy pipeline containing a nested threshold set flattened the nesting and reset every union operator to AND. Three further deviations (`-D3`, `-D5`, `-D6`) record SIMPLNX-only capabilities with no legacy equivalent — multi-component index selection, a selectable mask output `DataType`, and custom TRUE/FALSE values. + +## Algorithm Relationship + +*Classification:* **Rewrite** + +*Evidence:* `SimplnxCoreLegacyUUIDMapping.hpp` maps **two distinct legacy SIMPL UUIDs** to this single SIMPLNX filter's UUID: + +``` +014b7300-cf36-5ede-a751-5faf9b119dae → MultiThresholdObjectsFilter // MultiThresholdObjects ("Threshold Objects") +686d5393-2b02-5c86-b887-dd81a8ae80f2 → MultiThresholdObjectsFilter // MultiThresholdObjects2 ("Threshold Objects (Advanced)") +``` + +`FromSIMPLJson()` correspondingly branches on which legacy UUID (or, for 6.4 pipelines lacking a UUID, which legacy class name) produced the incoming JSON: the basic `MultiThresholdObjects` source is read through `ComparisonSelectionFilterParameterConverter` (flat, AND-only comparison list) and the advanced `MultiThresholdObjects2` source is read through `ComparisonSelectionAdvancedFilterParameterConverter` (nested AND/OR comparison sets). Both are converted into the same `ArrayThresholdSet` argument. This is **not** a line-by-line port of a single legacy algorithm — it's a consolidation of two independently-shipped legacy filters into one, which is why the classification is **Rewrite** rather than Port, per `vv_policy.md`: *"keeping [a UUID relationship] is a claim of functional equivalence... The Deviations file must defend the claim."* Here the claim is stronger than usual — that the merged filter reproduces each of the two legacy filters' behavior when configured equivalently to it. SIMPL 6.4/6.5 conversion fixtures exist at `test/simpl_conversion/6_4/MultiThresholdObjectsFilter.json` and `test/simpl_conversion/6_5/MultiThresholdObjectsFilter.json`, asserting the *argument conversion* round-trips correctly (also now covered by the `SIMPL Backwards Compatibility` `TEST_CASE`). Execution-output equivalence against both legacy filters has now been runtime-A/B-verified on representative flat, nested, and inverted-nested configurations — see Deviations file for the comparison record. + +*Structural differences from each legacy source:* + +1. **Consolidation itself** — one `ArrayThresholdSet` tree replaces two separate legacy parameter models (flat list vs. nested set); the flat legacy model is representable as a one-level `ArrayThresholdSet`. Spot-verified equivalent via runtime A/B (`AB1`, flat config vs. legacy `Threshold Objects`) — see Deviations file. +2. Multi-component index selection added (`#1184`, `32837a30f`) — confirmed **NX-only**: legacy `Threshold Objects (Advanced)`'s `dataCheck()` rejects non-scalar arrays outright (error `-11003`); legacy `Threshold Objects` never had per-component comparison either. Documented as `MultiThresholdObjectsFilter-D3` (non-bug capability addition) in the Deviations file. +3. Custom TRUE/FALSE mask output values added (`#669`, `b65210cf3`) — additive parameter; unconfirmed whether either legacy filter had this option or if it's SIMPLNX-only. Execution-time application (not just preflight bounds-checking) is now covered by the `Valid Execution - Custom Values` `TEMPLATE_TEST_CASE`. +4. Default mask output `DataType` changed to `uint8` (`#1502`, `49919b086`) — unconfirmed against either legacy filter's default. +5. `executeImpl()` body moved into `Algorithms/MultiThresholdObjects.{hpp,cpp}` (`#1544`, `8381d1dd5`) — structural only, no behavior change (internal to SIMPLNX, not a legacy-relationship concern). + +*Material PRs since baseline (2025-10-01):* + +- **#1582** — "ENH: Add missing cancel checks to lots of filters" (`1a42ec6fb`) — cross-cutting PR; added `m_ShouldCancel` checks to many filters including this one (visible today in `MultiThresholdObjects::operator()` and `ComputeThresholdSet`'s per-child cancel check). No output-behavior change on a non-cancelled run. +- **#1605** — "BUG: Fix SIMPL JSON conversion segfault and re-enable backwards-compatibility checks" (`996d7af5a`) — fixed a crash in `FromSIMPLJson()` and re-enabled the SIMPL 6.4/6.5 backwards-compatibility test for this filter (now `SIMPL Backwards Compatibility` in the test file). Affects pipeline-conversion correctness, not execution output. +- This PR itself also rewrote the comparison/combination internals, fixing two bugs — `MultiThresholdObjectsFilter-D1` and `-D2` — see Deviations file. The elementwise comparison itself is unchanged from legacy: `ThresholdFilterHelper::filterDataWithComparision` truncates the comparison value to the input array's type and compares with `std::less`/`std::greater`/`std::equal_to`/`std::not_equal_to` in that type, exactly as `SIMPL/Source/SIMPLib/Filtering/ThresholdFilterHelper.h:60` does (see Oracle → *Comparison-value truncation*). + +*Port-time deltas introduced by this PR* (each stated with whether it changes output): + +1. **Combination path consolidated.** The dual apply strategy (direct copy for the first item in a set, AND/OR combine for later items) and the redundant inversion parameter were replaced by a single `ComputeThresholdSet` → `ApplyThresholdValues` → `InsertThreshold` path. **Changes output** — this is the D1/D2 fix, and it restores legacy semantics. +2. **Component-count preflight relaxed.** The check requiring every thresholded array to have the same component count is removed, along with its error code `ErrorCodes::UnequalComponents` (`-4001`) in `MultiThresholdObjectsFilter.hpp`. A threshold set may now mix arrays with different component counts; each threshold selects its own `componentIndex`, and only the *tuple* counts must agree (still enforced via `ErrorCodes::UnequalTuples`). **Does not change output** for any pipeline that previously passed preflight — it widens what is accepted. Pipelines that previously failed with `-4001` now preflight and execute. Removing the enumerator from the public `ErrorCodes` enum is a source-level API break for any out-of-tree plugin referencing `MultiThresholdObjectsFilter::ErrorCodes::UnequalComponents`; nothing in this repository does (verified by grep across `src/` and the plugin tree). +3. **Intermediate storage moved from `std::vector` to `AbstractDataStore`.** One store per nesting level, allocated through `DataStoreUtilities::CreateDataStore`. **Does not change output** — an out-of-core-readiness change only. +4. **SIMPL nested-set conversion no longer flattens.** `ComparisonSelectionAdvancedFilterParameterConverter` now preserves nested `ArrayThresholdSet`s and sets each leaf's union operator from the legacy JSON, instead of collapsing the tree and defaulting every leaf to AND. **Changes output** for converted legacy pipelines — this is the `MultiThresholdObjectsFilter-D4` fix. Added here rather than deferred because the nested-set support this PR adds to the algorithm was unreachable from a converted legacy pipeline without it. + +## Oracle + +*Class:* **1 (Analytical)** + +*Applied:* For a single threshold, `expected[i] = COMPARISON(input[i], value)` (optionally inverted); for a component-indexed array, `input[i]` is replaced by `input[i][componentIndex]`. For a threshold set, `expected` is the boolean combination of each member's own `expected` value: the first member always seeds the accumulator, and the configured `UnionOperator` (AND/OR) combines each subsequent member; the whole set's `expected` is inverted again if the set itself is marked inverted. Every free variable in this formula — comparison operator, invert flag, union operator, set nesting, component index, mask output `DataType`, source-array `DataType`, and custom TRUE/FALSE execution values — is enumerated directly against this closed-form definition in the test file's `Expected*Mask` helper functions and inline hardcoded exemplar arrays, independent of the algorithm's own C++ control flow (`ThresholdFilterHelper`, `InsertThreshold`, `ApplyThresholdValues`). + +*Comparison-value truncation (verified, legacy-matching):* the user-supplied comparison value is a `float64`, but `ThresholdFilterHelper::filterDataWithComparision` (`Algorithms/MultiThresholdObjects.cpp`) truncates it to the input array's type and performs the comparison in that type. A threshold of `5.5` against an `int32` array therefore compares against `5`, so `5 == 5.5` is **true** for this filter. This matches legacy DREAM3D exactly — `SIMPL/Source/SIMPLib/Filtering/ThresholdFilterHelper.h:60` performs the identical `static_cast(m_ComparisonValue)` — and it keeps 64-bit integer comparisons exact, which comparing in `float64` would not (values above 2^53 differing by 1 would collapse to equal). The `Expected*Mask` oracle helpers model the same truncation (`static_cast(thresholdValue)` for the int fixtures, `static_cast(...)` on both operands for the `Float32Array` fixture) so the oracle and the implementation agree by construction rather than by coincidence of fixture size. `Valid Single Thresholds: Int` deliberately `GENERATE`s a fractional threshold (`5.5`) against integer data to keep this behavior asserted. + +*Encoded:* `test/MultiThresholdObjectsTest.cpp` — + +- `Exemplar Single Thresholds: Int` / `: Float` — single fixed-threshold fixtures checked against hardcoded `k_ExemplarInt4` / `k_ExemplarFloat02` arrays +- `Valid Single Thresholds: Int` / `: Float` / `: Int Multi-Component` — comparison operator × invert × (component index for multi-component) sweep, via `ExpectedIntSingleComponentMask` / `ExpectedFloatSingleComponentMask` / `ExpectedIntMultiComponentMask` +- `Valid Threshold Sets` — 7 hand-built configurations: AND / OR / nested-set / nested-set-with-OR / nested-set-with-OR+invert (`CreateThresholdSet1`–`5`, via `ExpectedThresholdSet1Mask`–`5`), plus the two mixed leaf-and-nested-set regression shapes `CreateThresholdSet6`/`7` (via `CheckThresholdSet6`/`7`) +- `Valid Execution, Mask DataType` — 10 mask-output `DataType`s +- `Valid Execution, Input Array DataType` — 11 source-array `DataType`s +- `Valid Execution - Custom Values` — 10 mask `DataType`s, custom TRUE/FALSE values applied at execution time (not just preflight bounds-checked) +- `SIMPL Backwards Compatibility` — SIMPL 6.4/6.5 argument-conversion round-trip +- `SIMPL Nested Set Conversion` — a converted legacy nested threshold set keeps its nesting, inversion flag and union operators +- `Invalid Execution`, `Invalid Execution - Out of Bounds Custom Values` (9 numeric types), `Invalid Execution - Boolean Custom Values` — negative-path fixtures + +14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations, all pass at HEAD. + +*Second-engineer review:* **Signed off by Michael A. Jackson , 2026-08-19** (PR #1688 review, per sign-off convention). The V&V work was authored by Matthew Marine, so the review is independent of the author. Reviewed across two passes (2026-07-24 and 2026-08-12) plus this closing pass: + +- **Oracle design.** Confirmed Class 1 is correct and non-circular — every fixture is built in memory, no `.dream3d` exemplar is consulted, and the `Expected*Mask` helpers re-derive the closed form rather than replaying the algorithm's control flow. Independently corroborated by a three-way A/B (DREAM3D 6.5.171 `PipelineRunner`, this branch's `nxrunner`, and an independent numpy oracle) matching exactly on flat, nested, and inverted-nested configurations at both 100-tuple and 50M-tuple scale — see the Deviations file. +- **Oracle/implementation reconciliation.** The oracle originally compared in `float64` while the implementation truncated the comparison value to the array type. The divergence was masked by the fixture holding only values `0`–`4`; raising the tuple count exposed it. Resolved in favor of the implementation, which matches legacy — the `Expected*Mask` helpers now model the truncation (see *Comparison-value truncation* above). +- **Test load-bearing check.** `Valid Execution, Mask DataType` was found to assert nothing (a hardcoded true/false split had gone stale against a reduced fixture, leaving the true branch unreachable and the expected mask all-false, which the zero-initialized output already satisfied). Verified fixed by stubbing `MultiThresholdObjects::operator()()` to `return {}` — all 10 `SECTION`s now fail. The same stub check was applied to the new `ArraySet 6` / `ArraySet 7` regression fixtures. +- **Code path coverage.** Row 25 was independently confirmed to be a genuine gap — every `CreateThresholdSet*` helper passed either all leaves or all nested sets, never a mix — and is now closed. +- **Closing pass — legacy comparison completed.** The three configurations previously carried as outstanding were closed: custom TRUE/FALSE values and the mask output `DataType` by direct inspection of the legacy sources (neither parameter exists in either legacy filter — recorded as `D6` and `D5`), and the broader configuration sweep by running `AB4`–`AB6` against the legacy `PipelineRunner`. Running those three through SIMPLNX's own SIMPL-pipeline conversion — a path no prior fixture had exercised — surfaced `MultiThresholdObjectsFilter-D4`, fixed here. + +## Code path coverage + +**26 of 27 paths exercised.** Row 13 is a permanent, acceptable gap and the only one remaining. Row 25 was the gap that let `MultiThresholdObjectsFilter-D1` ship and row 27 the gap that let `-D4` ship; both are now covered — see `vv/deviations/MultiThresholdObjectsFilter.md`. + +Source: `src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/MultiThresholdObjects.cpp` (~320 lines), plus 7 preflight-only paths in `src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MultiThresholdObjectsFilter.cpp` and the SIMPL-conversion path in `src/simplnx/Parameters/ArrayThresholdsParameter.cpp`. + +Three logical stages: **(a) preflight** validates the threshold set / mask-type / custom-value configuration and stages the output `CreateArrayAction`; **(b) algorithm** recursively evaluates the `ArrayThresholdSet` tree into an internal `bool` mask (per-array comparison combined inline by `ThresholdFilterHelper`, which selects an AND / OR / replace loop once per threshold, then `InsertThreshold`/`ApplyThresholdValues` for cross-node combination) and writes the result into the typed mask array via `ThresholdSetFunctor`, substituting `trueValue`/`falseValue` at that final write; **(c) SIMPL conversion** maps a legacy `MultiThresholdObjects` / `MultiThresholdObjects2` pipeline onto this filter's arguments via `FromSIMPLJson()` and the `ArrayThresholdsParameter` converters. + +| # | Stage | Path | Test case | +|----|-------------------|------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------| +| 1 | (a) Preflight | `thresholdPaths.empty()` → error `-4000` | `Invalid Execution` — "Empty ArrayThresholdSet" | +| 2 | (a) Preflight | Tuple-count mismatch across threshold arrays → `ErrorCodes::UnequalTuples` | `Invalid Execution` — "Mismatching Tuples in Threshold Arrays" | +| 3 | (a) Preflight | `componentIndex >= numComponents` (via `CheckComponentIndicesInThresholds`, recurses into nested sets) → `ErrorCodes::InvalidComponentIndex` | `Invalid Execution` — "Out of Bounds Component Index" | +| 4 | (a) Preflight | `maskArrayType == boolean && useCustomTrueValue` → `CustomTrueWithBoolean` | `Invalid Execution - Boolean Custom Values` — "Custom True Value" | +| 5 | (a) Preflight | `maskArrayType == boolean && useCustomFalseValue` → `CustomFalseWithBoolean` | `Invalid Execution - Boolean Custom Values` — "Custom False Value" | +| 6 | (a) Preflight | `useCustomTrueValue` value outside `[min,max]` of mask type → `CustomTrueOutOfBounds` | `TEMPLATE_TEST_CASE …Out of Bounds Custom Values` — "True Value < Minimum" / "> Maximum", all 9 numeric types | +| 7 | (a) Preflight | `useCustomFalseValue` value outside bounds → `CustomFalseOutOfBounds` | same — "False Value < Minimum" / "> Maximum" | +| 8 | (a) Preflight | Success → stage `CreateArrayAction` for mask output | every "Valid …" test | +| 9 | (b) Algorithm | `ComparisonType::LessThan` | "ArrayThreshold: <" (int / float / multi-component) | +| 10 | (b) Algorithm | `ComparisonType::GreaterThan` | "ArrayThreshold: >" | +| 11 | (b) Algorithm | `ComparisonType::Operator_Equal` | "ArrayThreshold: ==" | +| 12 | (b) Algorithm | `ComparisonType::Operator_NotEqual` | "ArrayThreshold: !=" | +| 13 | (b) Algorithm | `throw std::runtime_error` for an unrecognized union operator, at the end of `ThresholdFilterHelper::filterDataWithComparision` | *Not directly tested. Unreachable via the public `UnionOperator` enum — both enumerators (`And`, `Or`) are exercised elsewhere.* | +| 14 | (b) Algorithm | `InsertThreshold` with `inverse == true` (flip before combine) | `isInverted = GENERATE(false, true)` in every single-threshold and threshold-set test | +| 15 | (b) Algorithm | `InsertThreshold` with `inverse == false` | same | +| 16 | (b) Algorithm | Combine with `UnionOperator::Or` | `CreateThresholdSet2` (threshold2 = Or), `CreateThresholdSet4`/`5` (nested-set union = Or) | +| 17 | (b) Algorithm | Combine with `UnionOperator::And` | `CreateThresholdSet1` (threshold2/3 = And), `CreateThresholdSet3` default nested And | +| 18 | (b) Algorithm | `ApplyThresholdValues` with `replaceInput == true` (the first item in a set overwrites the accumulator, ignoring its configured union operator) | implicit in every threshold set — first entry of every `CreateThresholdSet*` | +| 19 | (b) Algorithm | `ApplyThresholdValues` with `replaceInput == false` (honors configured operator for later items) | same sets, 2nd/3rd entries | +| 20 | (b) Algorithm | `ComputeThresholdSet` recursion — item is a nested `ArrayThresholdSet` | `CreateThresholdSet3`/`4`/`5` (set-of-sets) | +| 21 | (b) Algorithm | `ComputeThresholdSet` — item is a leaf `ArrayThreshold` | all tests | +| 22 | (b) Algorithm | `ThresholdSetFunctor` dispatch on **mask (output) DataType** | `Valid Execution, Mask DataType` — int8/16/32/64 + uint8/16/32/64 + float32/64 (10 types; no `boolean` `SECTION` in this test — boolean is exercised separately via the default mask type used throughout `RunThresholdSetTest`/other `TEST_CASE`s) | +| 23 | (b) Algorithm | `ExecuteThresholdHelper` dispatch on **source array's DataType** | `Valid Execution, Input Array DataType` — int8/16/32/64 + uint8/16/32/64 + float32/64 + boolean, all 11 types | +| 24 | (b) Algorithm | Multi-component `componentIndex != 0` selection | `Valid Single Thresholds: Int Multi-Component` (`componentIndex = GENERATE(0,1,2)`), plus `componentIndex=1` in Set1, `=0` in Set2 | +| 25 | (b) Algorithm | An `ArrayThresholdSet` whose children mix at least one leaf `ArrayThreshold` with at least one nested `ArrayThresholdSet` (e.g. `{leaf, nestedSet}`, not `{leaf, leaf, leaf}` or `{set, set}`). Historically produced an all-false mask regardless of input (`MultiThresholdObjectsFilter-D1`, confirmed against legacy `Threshold Objects (Advanced)` — 38/100 tuples wrong on `develop`), fixed by this PR. | `Valid Threshold Sets` → `ArraySet 6: leaf + nested set` (`CreateThresholdSet6`, a leaf `Int > 2` sibling to a nested set) and `ArraySet 7: leaf + inverted nested set` (`CreateThresholdSet7`, the `AB3` shape), each × `isInverted`. Both were confirmed to fail when `MultiThresholdObjects::operator()()` is stubbed to `return {}` — i.e. they detect the all-false mask that D1 produced. Also confirmed externally by the `AB2`/`AB3` legacy A/B fixtures (see Deviations file), which are not part of the ctest suite. | +| 26 | (b) Algorithm | Custom TRUE/FALSE value substitution at execution time (`ThresholdSetFunctor` writing `trueValue`/`falseValue` from `MultiThresholdObjects::operator()`, not the default `1.0`/`0.0`) — distinct from rows 6–7, which only cover the preflight bounds-check rejecting *out-of-range* custom values and never actually execute with valid ones | `Valid Execution - Custom Values` (`TEMPLATE_TEST_CASE`, 10 numeric mask types), asserting `trueValue`/`falseValue` (25/10 in the test) appear in the output instead of 1/0 | +| 27 | (c) SIMPL conversion | `ComparisonSelectionAdvancedFilterParameterConverter` on a legacy pipeline containing a *nested* `ComparisonSet` — nesting, inversion flag and per-leaf union operators must all survive conversion (`MultiThresholdObjectsFilter-D4`) | `SIMPL Nested Set Conversion`, plus the `AB4`–`AB6` converted-pipeline runs recorded in the Deviations file | + +Not counted as an algorithm/preflight path: the "Empty ArrayThreshold DataPath" section of `Invalid Execution` exercises `ArrayThresholdsParameter`'s own path-existence validation, which runs before `preflightImpl` is called — it's a parameter-layer gate, not code inside this filter or algorithm. + +`MultiThresholdObjectsFilter-D2` (the pre-fix `std::reverse` tuple-order bug) does not get its own row: the buggy code path no longer exists on this branch, and its trigger — a leaf combined with an *inverted* nested set (the legacy A/B's `AB3` shape) — overlaps with row 25's mixed-sibling shape rather than isolating D2 cleanly on its own. It is covered in-repo by the `ArraySet 7` `SECTION` listed against row 25. + +## Test inventory + +| Test case | Status | Notes | +|-----------|--------|-------| +| `Exemplar Single Thresholds: Int` | kept | Single fixed threshold (`Int32 > 4`) checked against hardcoded `k_ExemplarInt4` array. | +| `Exemplar Single Thresholds: Float` | kept | Single fixed threshold (`Float32 == 0.02`) checked against hardcoded `k_ExemplarFloat02` array. | +| `Valid Single Thresholds: Int` | kept | `GENERATE` over 8 threshold values × 2 invert states, 4 `SECTION`s (`>`, `<`, `==`, `!=`) against `k_TestArrayIntPath`; every tuple checked via `ExpectedIntSingleComponentMask`. Modified this cycle: `ExpectedIntSingleComponentMask` now truncates the threshold to `int32` so the oracle matches the implementation's (and legacy's) comparison-value cast — the `5.5` case is what exposed the mismatch. The complementary-operator re-assertions in the `==` / `!=` `SECTION`s were dropped; they restated the oracle rather than testing the filter. | +| `Valid Single Thresholds: Float` | kept | Same sweep against `k_TestArrayFloatPath` via `ExpectedFloatSingleComponentMask`. Modified this cycle: the oracle rounds both operands through `float32` to match the `Float32Array` fixture, and the complementary-operator re-assertions were dropped for the same reason as the `Int` case. | +| `Valid Single Thresholds: Int Multi-Component` | kept | Adds `componentIndex = GENERATE(0,1,2)` against `k_MultiComponentArrayPath`. Modified this cycle with the same oracle-truncation and re-assertion changes as the single-component `Int` case. | +| `Valid Threshold Sets` | kept | 7 `SECTION`s, each × `isInverted`. `ArraySet 1`–`5` cover AND, OR, nested-set, nested-set-with-OR, and nested-set-with-OR+invert combinations. `ArraySet 6` / `ArraySet 7` were **added new-for-V&V**: a leaf threshold with a sibling nested set, and the same with the nested set inverted — the `MultiThresholdObjectsFilter-D1` / `-D2` trigger shapes (code-path row 25). Both were confirmed load-bearing by stubbing the algorithm to `return {}`. | +| `Invalid Execution` | kept | 4 `SECTION`s: empty threshold set (`-4000`), empty threshold `DataPath` (parameter-layer validation), out-of-bounds component index (`InvalidComponentIndex`), mismatched tuple counts (`UnequalTuples`). | +| `Invalid Execution - Out of Bounds Custom Values` (`TEMPLATE_TEST_CASE`) | kept | 9 numeric-type instantiations × 4 `SECTION`s (true/false value below minimum / above maximum) — `CustomTrueOutOfBounds` / `CustomFalseOutOfBounds`. | +| `Invalid Execution - Boolean Custom Values` | kept | 2 `SECTION`s — custom TRUE/FALSE value rejected when mask type is `boolean`. | +| `Valid Execution, Mask DataType` | kept | 10 `SECTION`s, one per mask-output `DataType` (int8…float64 — no `boolean` `SECTION`; boolean covered via the default mask type used throughout the other `TEST_CASE`s). Modified this cycle: the expected true/false split had been left as a hardcoded literal against a reduced fixture, which made the true branch unreachable and the expected mask all-false — the test asserted nothing. The split is now derived from the fixture constants with a `static_assert` that it lies inside the tuple range. | +| `Valid Execution, Input Array DataType` | kept | 11 `SECTION`s, one per **source-array** `DataType` (int8…float64, bool). | +| `SIMPL Nested Set Conversion` | new-for-V&V | Converts a legacy `MultiThresholdObjects2` pipeline whose thresholds are a leaf sibling to an *inverted* nested set with OR-joined children, and asserts the converted `ArrayThresholdSet` keeps the nesting, the inversion flag, the swapped comparison operators, and the children's union operators. Added for `MultiThresholdObjectsFilter-D4`; verified load-bearing (fails against the pre-fix converter). Fixture: `test/simpl_conversion/6_5/MultiThresholdObjectsFilter_Nested.json`. | +| `SIMPL Backwards Compatibility` | kept | `DYNAMIC_SECTION` over the SIMPL 6.4 and 6.5 conversion fixtures; asserts pipeline conversion round-trips (UUID + one argument value). Re-enabled by `#1605` after a prior segfault. Deleted earlier in this PR and restored during V&V review, so the `#1605` coverage is not lost. | +| `Valid Execution - Custom Values` (`TEMPLATE_TEST_CASE`) | kept | 10 numeric-type instantiations; asserts custom TRUE (`25`) / FALSE (`10`) values are actually written to the output mask at execution time — closes the code-path gap on row 26. Deleted earlier in this PR and restored during V&V review; `UnitTest::CheckArraysInheritTupleDims` was added at the same time. | + +**Missing:** nothing outstanding. The last gap — a set mixing a leaf threshold with a sibling nested set (row 25), the shape that triggered `MultiThresholdObjectsFilter-D1` — is closed by the `ArraySet 6` / `ArraySet 7` `SECTION`s of `Valid Threshold Sets`. + +**Count basis:** 31 ctest entries = 12 single-entry `TEST_CASE`s (1 ctest entry each) + `Invalid Execution - Out of Bounds Custom Values` (`TEMPLATE_TEST_CASE`, 9 types → 9 entries) + `Valid Execution - Custom Values` (`TEMPLATE_TEST_CASE`, 10 types → 10 entries), verified by directly reading each declaration's type list twice. `catch_discover_tests` is called with no filtering options in `cmake/Plugin.cmake:404`, so Catch2's default behavior applies: one ctest entry per `TEMPLATE_TEST_CASE` type instantiation. A count of 28 entries cited during review was correct at the commit it was measured against; the two `Exemplar Single Thresholds` `TEST_CASE`s added afterwards account for the difference. 31 matches the test file as it stands today. + +**Dual-build verification at sign-off:** `ctest -R MultiThresholdObjects` passes 31/31 in **both** builds — in-core (`NX-Com-Qt69-Vtk96-Rel`) and out-of-core (`NX-OOC-Qt69-Vtk95-Rel`). The full `SimplnxCore::` suite also passes 984/984 in-core. The OOC run initially reported all entries failing; all were `LoadPlugins()` aborting because stale plugin binaries in that build directory linked an older TBB soname, and all cleared after rebuilding those plugins — no failure was attributable to this filter. + +## Exemplar archive + +None. All fixtures for this filter are constructed in-memory in `test/MultiThresholdObjectsTest.cpp` (`CreateTestDataStructure()`, `CreateTestDataStructure2()`); there is no `.dream3d` exemplar and no `download_test_data()` entry in `test/CMakeLists.txt` for `MultiThresholdObjectsFilter`. No provenance sidecar is needed. + +## Deviations from DREAM3D 6.5.171 + +Legacy comparison **run and complete**: independent three-way A/B (DREAM3D 6.5.171 `PipelineRunner`, this branch's `nxrunner`, and a numpy oracle) across six configurations — flat, nested, inverted-nested, three-level nested, inverted sibling set, and a grouping-sensitive case — at 100 tuples, with the first three re-run at 50M tuples. All sources MATCH in every case. No configurations remain outstanding. Full record in `vv/deviations/MultiThresholdObjectsFilter.md`. + +- `MultiThresholdObjectsFilter-D1` — a set mixing a leaf threshold with a sibling nested set produced an all-false mask on `develop` (38/100 tuples wrong vs. legacy `Threshold Objects (Advanced)`). **Fixed by this PR**, with in-repo regression coverage (`Valid Threshold Sets` → `ArraySet 6`). +- `MultiThresholdObjectsFilter-D2` — a leaf combined with an inverted nested set used `std::reverse` to flip tuple order instead of flipping each tuple's value on `develop` (51/100 tuples wrong vs. the same legacy filter). **Fixed by this PR**, with in-repo regression coverage (`Valid Threshold Sets` → `ArraySet 7`). +- `MultiThresholdObjectsFilter-D3` — multi-component index selection is SIMPLNX-only; legacy `Threshold Objects (Advanced)` rejects non-scalar arrays (`dataCheck()` error `-11003`). Not a bug — a deliberate SIMPLNX capability addition, documented for migration guidance. +- `MultiThresholdObjectsFilter-D4` — converting a legacy pipeline containing a nested threshold set flattened the nesting and reset every union operator to AND, so a converted `Threshold Objects (Advanced)` pipeline computed a plain AND over all leaves (55/100, 50/100 and 4/100 tuples wrong on `AB4`–`AB6`). Pre-existing on `develop`, in `src/simplnx/Parameters/ArrayThresholdsParameter.cpp`. **Fixed by this PR**, with in-repo regression coverage (`SIMPL Nested Set Conversion`). +- `MultiThresholdObjectsFilter-D5` — the mask output `DataType` is selectable in SIMPLNX and defaults to `uint8`; both legacy filters always created a `DataArray`. Values are identical, the array type is not. Not a bug — set **Mask Type** to `boolean` when bit-for-bit legacy parity of the output file matters. +- `MultiThresholdObjectsFilter-D6` — custom TRUE/FALSE mask values are SIMPLNX-only; neither legacy filter has the parameter. Not a bug; the defaults (1/0) reproduce legacy behavior. + diff --git a/src/Plugins/SimplnxCore/vv/deviations/MultiThresholdObjectsFilter.md b/src/Plugins/SimplnxCore/vv/deviations/MultiThresholdObjectsFilter.md new file mode 100644 index 0000000000..8b4c516aa6 --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/deviations/MultiThresholdObjectsFilter.md @@ -0,0 +1,190 @@ +# Deviations from DREAM3D 6.5.171: MultiThresholdObjectsFilter + +This file lists every documented behavioral difference between this SIMPLNX filter and its DREAM3D 6.5.171 equivalent. + +Entries are referenced by stable ID (`MultiThresholdObjectsFilter-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. + +## Filter UUID + +`4246245e-1011-4add-8436-0af6bed19228` + +## Headline + +**6 deviations documented: 3 bugs (all in SIMPLNX, all fixed by this PR, all with in-repo regression tests), 3 confirmed non-bug capability differences.** Legacy comparison has been **run**: an independent three-way A/B — DREAM3D 6.5.171 `PipelineRunner`, this branch's `nxrunner`, and an independent numpy oracle — on a shared 100-tuple fixture, covering representative flat (`Threshold Objects`), nested, and inverted-nested (`Threshold Objects (Advanced)`) configurations, re-run again at 50M tuples. Post-fix, all three sources MATCH in every case at both scales. The in-repo `MultiThresholdObjectsTest.cpp` suite (14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations / 31 ctest entries — see the V&V report's Test inventory for the count basis) also passes locally. + +The same three pipelines run against `develop` (pre-fix) reproduce two real bugs quantitatively: `MultiThresholdObjectsFilter-D1` (38/100 tuples wrong) and `MultiThresholdObjectsFilter-D2` (51/100 tuples wrong). Both are **fixed by this PR** — not by a commit that predates this V&V pass. Both now have dedicated in-repo regression coverage as well: the `ArraySet 6` and `ArraySet 7` `SECTION`s of `Valid Threshold Sets` (see the V&V report's Code path coverage row 25 and Test inventory), each confirmed to fail when the algorithm is stubbed out. + +`MultiThresholdObjectsFilter-D3`, `-D5`, and `-D6` document confirmed, deliberate capability differences (not bugs): multi-component index selection, custom TRUE/FALSE mask values, and a selectable mask output `DataType` all exist only in SIMPLNX. + +`MultiThresholdObjectsFilter-D4` is a third SIMPLNX bug, found during second-engineer review and fixed by this PR: converting a legacy *nested* threshold set flattened it and discarded every union operator, so converted `Threshold Objects (Advanced)` pipelines computed a plain AND over all leaves. + +--- + +## Comparison method + +| | | +|---|---| +| **Comparison type** | Runtime three-way A/B: legacy DREAM3D 6.5.171 (`PipelineRunner`) vs. this branch (`nxrunner`) vs. an independent numpy oracle | +| **Shared input** | Legacy-format fixture, 100 tuples: `Int32 = 0..99`, `Float32 = 0.01*(i+1)` | +| **Scale re-run** | Same three configurations (AB1–AB3) re-run at 50M random tuples — this branch matches the numpy oracle exactly at scale | +| **Deeper-nesting fixture** | Second shared 100-tuple fixture generated by the legacy runner itself (`CreateDataArray`, random-with-range: `Int32` in [0,100), `Float32` in [0,1)), read by both runners from that same file. Used for `AB4`–`AB6` | +| **In-repo regression suite** | `test/MultiThresholdObjectsTest.cpp` passes locally at the verified commit (14 `TEST_CASE`/`TEMPLATE_TEST_CASE` declarations / 31 ctest entries; see the V&V report's Test inventory for the count basis) | + +### Per-configuration result (100-tuple fixture, this branch = post-fix) + +| Case | Config | Legacy filter | This branch vs. legacy vs. oracle | +|---|---|---|---| +| `AB1` | `Int32 > 42 AND Float32 < 0.70` (flat) | `MultiThresholdObjects` ("Threshold Objects") | **MATCH** (legacy = NX = oracle) | +| `AB2` | `Int32 > 20 AND (Float32 < 0.60 OR Int32 == 55)` (nested set) | `MultiThresholdObjects2` ("Threshold Objects (Advanced)") | **MATCH** | +| `AB3` | `Int32 < 80 OR NOT(Int32 > 30 AND Float32 < 0.95)` (inverted nested set) | `MultiThresholdObjects2` | **MATCH** | + +### Deeper-nesting configurations (added at second-engineer review, 2026-08-19) + +These close the "broader configuration sweep" item that was previously outstanding. Each was run three ways — legacy `MultiThresholdObjects2`, a natively-authored SIMPLNX pipeline, and an independent numpy oracle written from the English description — and a fourth way, through SIMPLNX's own conversion of the legacy SIMPL pipeline JSON. + +| Case | Config | Shape | Legacy vs. oracle | SIMPLNX native vs. oracle | SIMPLNX converted-from-SIMPL vs. oracle | +|---|---|---|---|---|---| +| `AB4` | `Int32 > 20 AND (Float32 < 0.8 OR (Int32 < 60 AND Float32 > 0.3))` | 3 levels deep | **MATCH** 100/100 | **MATCH** 100/100 | **MATCH** 100/100 (45/100 before the `D4` fix) | +| `AB5` | `(Int32 > 30 OR Float32 < 0.2) AND NOT(Int32 < 70 AND Float32 > 0.4)` | two sibling sets, second inverted | **MATCH** 100/100 | **MATCH** 100/100 | **MATCH** 100/100 (50/100 before) | +| `AB6` | `Int32 < 50 AND (Float32 > 0.9 OR Int32 == 3)` | grouping-sensitive, only 4 true tuples | **MATCH** 100/100 | **MATCH** 100/100 | **MATCH** 100/100 (96/100 before — an all-false mask) | + +`AB6` is deliberately built so the correct answer (4 true tuples) and the answer produced by discarding the AND/OR grouping (0 true tuples) differ, so it fails loudly if grouping is not honored. + +### Pre-fix (`develop`) result, same three pipelines + +| Case | Result on `develop` (pre-fix) | Deviation | +|---|---|---| +| `AB1` | Matches legacy/oracle (the flat config was never affected — see D1/D2 root causes) | none | +| `AB2` | **All-false mask — 38/100 tuples wrong** | `MultiThresholdObjectsFilter-D1` | +| `AB3` | **Differs in 51/100 values** | `MultiThresholdObjectsFilter-D2` | + +Both bug-fix claims in the PR are real, and the fix restores legacy semantics: legacy's `invertThreshold()` flips values element-wise; the old SIMPLNX `std::reverse` was a misport of that, and the old per-item functor dispatch broke nested-set combination entirely. (Engineer's account.) + +--- + +## MultiThresholdObjectsFilter-D1 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D1` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active (SIMPLNX bug **fixed during this V&V cycle** in PR #1688; documented for users of prior SIMPLNX releases) | + +**Symptom:** On `develop`, an `ArrayThresholdSet` whose children mix at least one leaf `ArrayThreshold` with at least one nested `ArrayThresholdSet` (e.g. `AB2`: `{leaf: Int32 > 20, nestedSet: (Float32 < 0.60 OR Int32 == 55)}`) produced an all-false mask, regardless of input data. Quantified on the `AB2` fixture: **38 of 100 tuples wrong** (all forced false) vs. legacy `Threshold Objects (Advanced)` and the numpy oracle. + +**Root cause:** Bug (SIMPLNX-side, `develop`). Per the reporting engineer: the old per-item functor dispatch broke nested-set combination entirely — a redundant inversion/apply mechanism was threaded separately from each node's own `isInverted()`, combined with a dual apply strategy (direct-copy for the first item in a set vs. an AND/OR combine for later items against an accumulator pre-filled false) that this PR replaced with a single, consistent combination path. The exact instruction-level trace of why this specifically zeroed the mask for the mixed-leaf/nested-set shape on `develop` was not independently re-derived line-by-line for this report; the symptom and fix are established by the `AB2` runtime comparison against real legacy output, which is stronger evidence than a re-derived trace would be. + +**Affected users:** Any pipeline (SIMPLNX-native, or converted from legacy `Threshold Objects (Advanced)`) using a threshold set that combines a plain leaf comparison with a sibling nested group — a common shape, not an exotic edge case. Silent on `develop`: the filter reported success and wrote a fully-false mask with no warning. + +**Regression coverage:** `Valid Threshold Sets` → `ArraySet 6: leaf + nested set` (`CreateThresholdSet6`/`CheckThresholdSet6` in `test/MultiThresholdObjectsTest.cpp`), swept over `isInverted`. Verified load-bearing: stubbing `MultiThresholdObjects::operator()()` to `return {}` — which reproduces D1's all-false mask, since the output array is zero-initialized at creation — makes this `SECTION` fail. + +**Recommendation:** Trust SIMPLNX (this PR — confirmed bit-for-bit against legacy on `AB2` at both 100 tuples and 50M tuples). The `develop` output was unconditionally wrong; anyone on a `develop` build predating this PR should upgrade and re-verify any pipeline outputs generated before the fix. + +--- + +## MultiThresholdObjectsFilter-D2 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D2` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active (SIMPLNX bug **fixed during this V&V cycle** in PR #1688; documented for users of prior SIMPLNX releases) | + +**Symptom:** On `develop`, a leaf combined with an inverted nested set (`AB3`: `{leaf: Int32 < 80, invertedNestedSet: NOT(Int32 > 30 AND Float32 < 0.95)}`) produced incorrect mask output. Quantified on the `AB3` fixture: **51 of 100 values differ** vs. legacy `Threshold Objects (Advanced)` and the numpy oracle. `AB3`'s shape overlaps with `D1`'s mixed-leaf/nested-set trigger, so this result is not a clean isolation of the inversion defect alone — both mechanisms plausibly contribute to the discrepancy. + +**Root cause:** Bug (SIMPLNX-side, `develop`). Per the reporting engineer: legacy's `invertThreshold()` flips mask values element-wise; the `develop` code instead called `std::reverse()` on the intermediate result buffer under certain replace/invert conditions — reversing the *order* of elements rather than flipping each element's own TRUE/FALSE value. This is not a valid implementation of per-element boolean inversion. This PR fixed it by consolidating all inversion through a single, consistent per-element-flip combination path (visible today as `InsertThreshold`'s `newVector.getValue(i) != inverse`). The precise nesting depth at which the `develop` `std::reverse` branch was reachable (top-level only, or also for a nested child, as in `AB3`) was not independently re-derived line-by-line for this report; documented here on the `AB3` runtime evidence. + +**Affected users:** Any pipeline using an inverted `ArrayThresholdSet` — top-level or nested — combined with sibling thresholds/sets. Not a narrow edge case: "Invert Mask" is a standard, documented option. On an image geometry, wrong tuple correspondence scrambles which voxels are masked; there is no legitimate downstream use of the `develop` output. + +**Regression coverage:** `Valid Threshold Sets` → `ArraySet 7: leaf + inverted nested set` (`CreateThresholdSet7`/`CheckThresholdSet7`), the in-repo analogue of the `AB3` shape, swept over `isInverted`. Verified load-bearing by the same stub experiment as D1. + +**Recommendation:** Trust SIMPLNX (this PR — confirmed bit-for-bit against legacy on `AB3` at both 100 tuples and 50M tuples). Anyone on a `develop` build predating this PR using an inverted threshold set should upgrade and re-verify any pipeline outputs generated before the fix. + +--- + +## MultiThresholdObjectsFilter-D3 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D3` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active | + +**Symptom:** SIMPLNX accepts a component index on a multi-component threshold array (`ArrayThreshold::setComponentIndex()`); neither legacy filter has an equivalent parameter. + +**Root cause:** Algorithmic choice (deliberate SIMPLNX capability addition, not a port artifact — appropriate under this filter's Rewrite classification). Confirmed directly from legacy source: `MultiThresholdObjects2::dataCheck()` rejects non-scalar (multi-component) arrays outright with error `-11003`. Legacy `Threshold Objects` (the non-Advanced filter) has no per-component comparison concept either. SIMPLNX's `#1184` (`32837a30f`) added multi-component index selection with no legacy equivalent in either predecessor. + +**Affected users:** Nobody migrating *from* legacy is affected (the capability didn't exist to lose). Anyone relying on this SIMPLNX-only feature should be aware there is no DREAM3D 6.5.171 equivalent pipeline to fall back to if downgrading. + +**Recommendation:** Trust SIMPLNX. This is an intentional superset capability, not a correctness issue. Worth a one-line callout in public migration guidance: legacy `Threshold Objects (Advanced)` never supported per-component thresholding on multi-component arrays; SIMPLNX does. + +--- + +## MultiThresholdObjectsFilter-D4 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D4` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active (SIMPLNX bug **fixed during this V&V cycle** in PR #1688; documented for users of prior SIMPLNX releases) | + +**Symptom:** A legacy `Threshold Objects (Advanced)` pipeline whose thresholds contain a *nested* comparison set produced the wrong mask after conversion to SIMPLNX. The nesting was discarded and every union operator was reset to AND, so `A AND (B OR C)` was evaluated as `A AND B AND C`. Quantified on the `AB4`–`AB6` fixtures against real legacy output: **55/100, 50/100, and 4/100 tuples wrong** respectively. `AB6` degenerated to an all-false mask. Filters run from natively-authored SIMPLNX pipelines were never affected — only pipelines converted from SIMPL. + +**Root cause:** Bug (SIMPLNX-side, pre-existing on `develop`; not introduced by this PR). Two independent defects in `src/simplnx/Parameters/ArrayThresholdsParameter.cpp`, both in the `ComparisonSelectionAdvancedFilterParameterConverter` path: + +1. `flattenSetThreshold()` collapsed each nested `ArrayThresholdSet` into its parent's flat threshold list. SIMPLNX represents nested sets natively, so the flattening served no purpose and destroyed the AND/OR grouping the legacy pipeline encoded. +2. `convertArrayThreshold()` never called `setUnionOperator()`, so every converted leaf silently took `ArrayThreshold`'s default (`And`). Any OR in the legacy pipeline was lost. This affected leaves at every level, not just nested ones. + +The flattening pass also carried a third latent defect: it attempted to push an inverted set's inversion down onto its leaves by flipping each comparison operator, which is not a valid negation — `NOT(x > v)` is `x >= v`, not `x < v`, so tuples exactly equal to the threshold were misclassified; De Morgan's required AND↔OR flip was never applied; and `Operator_NotEqual` fell through unchanged, so `NOT(x != v)` stayed `x != v`. + +**Fix:** the converter now keeps the nested `ArrayThresholdSet` (which `convertSetThreshold()` was already building correctly) and sets each leaf's union operator from the legacy `"Union Operator"` key. `flattenSetThreshold()` and `invertComparison()` are removed. Set inversion is handled natively by the algorithm, which is what this PR's rewrite made correct. + +**Affected users:** Anyone opening or running a converted DREAM3D 6.5 `Threshold Objects (Advanced)` pipeline that used a nested comparison set or any OR union — a standard use of that filter, and the only reason to choose the Advanced variant over the basic one. The failure was silent: the filter reported success and wrote a wrong mask. Natively-authored SIMPLNX pipelines were never affected. + +**Regression coverage:** `SIMPL Nested Set Conversion` (`test/MultiThresholdObjectsTest.cpp`, fixture `test/simpl_conversion/6_5/MultiThresholdObjectsFilter_Nested.json`) asserts that a converted leaf-plus-inverted-nested-set survives as a nested `ArrayThresholdSet` with its inversion flag and its children's union operators intact. Verified load-bearing: the test fails against the pre-fix converter. + +**Recommendation:** Trust SIMPLNX (this PR — confirmed against legacy on `AB4`–`AB6`). Any pipeline converted from SIMPL before this PR that used nested sets or OR unions should be re-converted and re-run; masks produced from such a converted pipeline should not be trusted. + +--- + +## MultiThresholdObjectsFilter-D5 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D5` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active | + +**Symptom:** SIMPLNX can write the mask with a user-selected `DataType` and defaults to `uint8`. Both legacy filters always created the mask as `DataArray` with no output-type option, so a converted 6.5 pipeline produces a `uint8` mask where legacy produced a `bool` mask. The mask *values* are identical (1/0); only the array's type differs. + +**Root cause:** Algorithmic choice (deliberate SIMPLNX capability addition). Confirmed directly from legacy source: `MultiThresholdObjects::dataCheck()` and `MultiThresholdObjects2::dataCheck()` both call `createNonPrereqArrayFromPath, AbstractFilter, bool>(...)` unconditionally. SIMPLNX added the `Mask Type` parameter with a `uint8` default (`#1502`). `FromSIMPLJson()` maps the SIMPL `ScalarType` key when present, but 6.5 pipelines do not carry that key, so a converted 6.5 pipeline falls through to the SIMPLNX default rather than to `boolean`. + +**Affected users:** Anyone converting a 6.5 pipeline whose downstream filters require a `boolean` mask array, or who compares a converted pipeline's output file against a legacy one and finds the mask array typed `uint8` instead of `bool`. Numerically the masks agree, so results are affected only where the array's declared type matters. + +**Recommendation:** Either acceptable. Users who need bit-for-bit legacy parity on the output file should set **Mask Type** to `boolean` explicitly after converting a 6.5 pipeline. Worth a line in public migration guidance. + +--- + +## MultiThresholdObjectsFilter-D6 + +| Field | Value | +|---|---| +| **Deviation ID** | `MultiThresholdObjectsFilter-D6` | +| **Filter UUID** | `4246245e-1011-4add-8436-0af6bed19228` | +| **Status** | active | + +**Symptom:** SIMPLNX can substitute custom TRUE and FALSE values into the mask instead of 1 and 0 (`Use Custom TRUE Value` / `Use Custom FALSE Value`). Neither legacy filter has any equivalent parameter. + +**Root cause:** Algorithmic choice (deliberate SIMPLNX capability addition, `#669`). Confirmed directly from legacy source: `MultiThresholdObjects::setupFilterParameters()` declares exactly two parameters — `SelectedThresholds` and `DestinationArrayName` — and `MultiThresholdObjects2` likewise has no custom-value parameter. Both write a hard-coded `true`/`false` into a `DataArray`. + +**Affected users:** Nobody migrating *from* legacy is affected — the capability did not exist to lose, and the defaults (1/0) reproduce legacy behavior exactly. Anyone relying on this SIMPLNX-only feature should know there is no DREAM3D 6.5.171 equivalent pipeline to fall back to. + +**Recommendation:** Trust SIMPLNX. Intentional superset capability, not a correctness issue. Because the custom values are substituted only at the final typed write, after the boolean combination is complete, they cannot affect which tuples are masked — the `Valid Execution - Custom Values` `TEMPLATE_TEST_CASE` covers all 10 numeric mask types analytically. + +## Outstanding comparison work + +**None.** Both legacy filters have been run separately on representative configurations (`AB1` vs. `Threshold Objects`; `AB2`–`AB6` vs. `Threshold Objects (Advanced)`), satisfying this filter's Rewrite-classification requirement that functional equivalence be confirmed against both predecessors. The three items previously listed here were closed at second-engineer review on 2026-08-19: + +1. **Custom TRUE/FALSE mask output values** — closed by inspection of the legacy sources rather than by an A/B run: neither legacy filter has the parameter, so there is nothing to compare against. Recorded as `MultiThresholdObjectsFilter-D6`. +2. **Default mask output `DataType`** — closed the same way: both legacy filters hard-code `DataArray`. Recorded as `MultiThresholdObjectsFilter-D5`, with migration guidance. +3. **A broader configuration sweep** — closed by running `AB4`–`AB6` (three levels of nesting, an inverted sibling set, and a grouping-sensitive case), all MATCH. This sweep is also what surfaced `MultiThresholdObjectsFilter-D4`. diff --git a/src/simplnx/Parameters/ArrayThresholdsParameter.cpp b/src/simplnx/Parameters/ArrayThresholdsParameter.cpp index 8a44eda714..1d94cfc6eb 100644 --- a/src/simplnx/Parameters/ArrayThresholdsParameter.cpp +++ b/src/simplnx/Parameters/ArrayThresholdsParameter.cpp @@ -209,21 +209,6 @@ bool isArrayThreshold(const nlohmann::json& json) return !json.contains(k_ThresholdValuesKey); } -ArrayThreshold::ComparisonType invertComparison(ArrayThreshold::ComparisonType comparison) -{ - switch(comparison) - { - case ArrayThreshold::ComparisonType::GreaterThan: - return ArrayThreshold::ComparisonType::LessThan; - case ArrayThreshold::ComparisonType::LessThan: - return ArrayThreshold::ComparisonType::GreaterThan; - case ArrayThreshold::ComparisonType::Operator_Equal: - return ArrayThreshold::ComparisonType::Operator_NotEqual; - default: - return comparison; - } -} - std::shared_ptr convertArrayThreshold(const DataPath& amPath, const nlohmann::json& json) { auto comparisonType = static_cast(json[k_ComparisonOperatorKey].get()); @@ -239,52 +224,18 @@ std::shared_ptr convertArrayThreshold(const DataPath& amPath, co auto daName = json[k_DataArrayNameKey].get(); auto daPath = amPath.createChildPath(daName); + // SIMPL and SIMPLNX agree on the union operator encoding (And = 0, Or = 1), so it carries over directly. + // The first threshold of a set has its union operator ignored by both implementations. + auto unionOperator = static_cast(json.value(k_UnionOperatorKey.view(), 0)); + auto value = std::make_shared(); value->setArrayPath(daPath); value->setComparisonType(comparisonType); value->setComparisonValue(comparisonValue); + value->setUnionOperator(unionOperator); return value; } -std::vector> flattenSetThreshold(const std::shared_ptr& thresholdSet, bool parentInverted = false) -{ - std::vector> flattenedArrays; - auto thresholds = thresholdSet->getArrayThresholds(); - if(thresholds.empty()) - { - return {}; - } - - bool inverted = thresholdSet->isInverted(); - if(parentInverted) - { - inverted = !inverted; - } - auto unionOperator = thresholdSet->getUnionOperator(); - - for(const auto& threshold : thresholds) - { - if(auto set = std::dynamic_pointer_cast(threshold); set != nullptr) - { - auto flattened = flattenSetThreshold(set, inverted); - flattenedArrays.insert(flattenedArrays.end(), flattened.begin(), flattened.end()); - } - else - { - auto arrayThreshold = std::dynamic_pointer_cast(threshold); - if(inverted) - { - auto comparisonType = invertComparison(arrayThreshold->getComparisonType()); - arrayThreshold->setComparisonType(comparisonType); - arrayThreshold->setUnionOperator(unionOperator); - } - flattenedArrays.push_back(arrayThreshold); - } - } - - return flattenedArrays; -} - std::shared_ptr convertSetThreshold(const DataPath& amPath, const nlohmann::json& json) { ArrayThresholdsParameter::ValueType::CollectionType thresholdSet; @@ -340,9 +291,9 @@ Result Compariso } else { - auto arrayThresholdSet = convertSetThreshold(amPath, iter); - auto flattenedThresholds = flattenSetThreshold(arrayThresholdSet); - thresholdSet.insert(thresholdSet.end(), flattenedThresholds.begin(), flattenedThresholds.end()); + // Preserve the nested set. SIMPLNX evaluates ArrayThresholdSet trees directly, so flattening here + // would discard the AND/OR grouping the legacy pipeline encoded. + thresholdSet.push_back(convertSetThreshold(amPath, iter)); } }