BUG: Fix validation logic in ValidateNumFeaturesInArray (#768)

As long as the max 'featureId' value is Less than the total number of tuples in the source attribute matrix, this is fine. That just means that there are unused features which is odd but not critical. Signed-off-by: Michael Jackson <[email protected]>
BlueQuartzSoftware · Nov 15, 2023 · f36fe62 · f36fe62
1 parent 2704410
commit f36fe62
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 31 deletions.
diff --git a/src/Plugins/ComplexCore/test/FindLargestCrossSectionsTest.cpp b/src/Plugins/ComplexCore/test/FindLargestCrossSectionsTest.cpp
@@ -248,6 +248,15 @@ DataStructure CreateValidTestDataStructure()
   return ds;
 }
 
+/**
+ * @brief
+ *  Create a Cell and Cell Feature attribute Matrix. The Cell AttributeMatrix will have a
+ *  featureIds DataArray that has values of 10. The Cell Feature AttributeMatrix will have
+ *  only 6 tuples in it. This means that the maximum value of FeatureIds would be 5. By having
+ *  values in the Feature Ids = 10, the preflight would pass but the execute would fail.
+ * @param geomIs3d
+ * @return
+ */
 DataStructure CreateInvalidTestDataStructure(bool geomIs3d)
 {
   DataStructure ds;
@@ -267,7 +276,7 @@ DataStructure CreateInvalidTestDataStructure(bool geomIs3d)
 
   auto* featureIds = Int32Array::CreateWithStore<Int32DataStore>(ds, k_FeatureIds, dims, {1}, cellAM->getId());
   auto& featureIdsDataStore = featureIds->getDataStoreRef();
-  featureIdsDataStore.fill(0);
+  featureIdsDataStore.fill(10);
   return ds;
 }
 } // namespace

diff --git a/src/complex/Utilities/DataArrayUtilities.cpp b/src/complex/Utilities/DataArrayUtilities.cpp
@@ -242,44 +242,25 @@ Result<> ValidateNumFeaturesInArray(const DataStructure& dataStructure, const Da
   {
     return MakeErrorResult(-5550, fmt::format("Could not find the input array path '{}' for validating number of features", arrayPath.toString()));
   }
-
+  Result<> results = {};
   const usize numFeatures = featureArray->getNumberOfTuples();
-  bool mismatchedFeatures = false;
-  usize largestFeature = 0;
+
   for(const int32& featureId : featureIds)
   {
-    if(static_cast<usize>(featureId) > largestFeature)
+    if(featureId < 0)
     {
-      largestFeature = featureId;
-      if(largestFeature >= numFeatures)
-      {
-        mismatchedFeatures = true;
-        break;
-      }
+      results = MakeErrorResult(
+          -5555, fmt::format("Feature Ids array with name '{}' has negative values within the array. The first negative value encountered was '{}'. All values must be positive within the array",
+                             featureIds.getName(), featureId));
+      return results;
     }
-  }
-
-  Result<> results = {};
-  if(mismatchedFeatures)
-  {
-    results = MakeErrorResult(-5551, fmt::format("The largest Feature Id {} in the FeatureIds array is larger than the number of Features ({}) in the Feature Data array at path '{}'", largestFeature,
-                                                 numFeatures, arrayPath.toString()));
-  }
-
-  if(largestFeature != (numFeatures - 1))
-  {
-    results =
-        MergeResults(results, MakeErrorResult(-5552, fmt::format("The number of Features ({}) in the Feature Data array at path '{}' does not match the largest Feature Id in the FeatureIds array {}",
-                                                                 numFeatures, arrayPath.toString(), largestFeature)));
-
-    const auto* parentAM = dataStructure.getDataAs<AttributeMatrix>(arrayPath.getParent());
-    if(parentAM != nullptr)
+    if(static_cast<usize>(featureId) >= numFeatures)
     {
-      results = MergeResults(results, MakeErrorResult(-5553, fmt::format("The input Attribute matrix at path '{}' has {} tuples which does not match the number of total features {}",
-                                                                         arrayPath.getParent().toString(), parentAM->getNumTuples(), largestFeature + 1)));
+      results = MakeErrorResult(-5551, fmt::format("Feature Ids array with name '{}' has a value '{}' that would exceed the number of tuples {} in the selected feature array '{}'",
+                                                   featureIds.getName(), featureId, numFeatures, arrayPath.toString()));
+      return results;
     }
   }
-
   return results;
 }