VV: Require Minimum Number of Neighbors fully V&V'ed

src/Plugins/SimplnxCore/docs/RequireMinNumNeighborsFilter.md

@@ -6,24 +6,76 @@ Processing (Cleanup)
 
 ## Description
 
-This **Filter** sets the minimum number of contiguous neighboring **Features** a **Feature** must have to remain in the structure. Entering zero results in nothing changing.  Entering a number larger than the maximum number of neighbors of any **Feature** generates an *error* (since all **Features** would be removed). The user needs to proceed conservatively here when choosing the value for the minimum to avoid accidentally exceeding the maximum. After **Features** are removed for not having enough neighbors, the remaining **Features** are *coarsened* iteratively, one **Cell** per iteration, until the gaps left by the removed **Features** are filled.  Effectively, this is an isotropic **Feature** growth in the regions around removed **Features**.
+This **Filter** removes **Features** that have fewer contiguous neighboring **Features** than the selected minimum. It uses a precomputed *Number of Neighbors* array, which is normally created by the [Compute Feature Neighbors](./ComputeFeatureNeighborsFilter.md) **Filter**. **Feature** tuple 0 is retained as the background tuple.
 
-The **Filter** can be run in a mode where the minimum number of neighbors is applied to a single **Ensemble**.  The user can select to apply the minimum to one specific **Ensemble**.
+When *Apply to Single Phase Only* is disabled, the minimum is applied to every non-background **Feature**. When it is enabled, only **Features** belonging to the selected *Phase Index* are tested against the minimum; **Features** in other **Ensembles** remain active.
+
+Cells belonging to removed **Features**, along with cells that already have a negative Feature ID, are reassigned iteratively. During each pass, every unresolved **Cell** examines its valid face neighbors and selects a neighbor associated with the most frequently occurring non-negative Feature ID. Vote ties are resolved by the face-neighbor traversal order. Every currently fillable **Cell** is updated during the pass, so the process may fill many cells per iteration. This produces an isotropic, face-connected growth of retained **Features** into the regions left by removed **Features**.
+
+After coarsening completes, inactive feature tuples are removed from the feature **Attribute Matrix** and the remaining Feature IDs are remapped.
+
+## Required Geometry and Inputs
+
+- An **Image Geometry**.
+- A scalar `int32` *Cell Feature Ids* **Data Array** containing exactly one tuple per geometry **Cell**.
+- A scalar `int32` *Number of Neighbors* **Data Array** containing one tuple per **Feature**.
+- When *Apply to Single Phase Only* is enabled, a scalar `int32` *Feature Phases* **Data Array** with the same tuple count as *Number of Neighbors*.
+- Every non-negative Feature ID must be less than the number of tuples in the feature **Attribute Matrix**. Negative Feature IDs are treated as unresolved cells and are reassigned during coarsening.
+
+## Data Modified by the Filter
+
+This **Filter** operates in place and does not create a new geometry or output array.
+
+- *Cell Feature Ids* is always updated, even if it is included in *Cell Arrays to Ignore*.
+- Every other nonignored **Data Array** in the cell **Attribute Matrix** copies tuple values from the same selected face neighbor used to reassign the Feature ID.
+- Arrays selected by *Cell Arrays to Ignore* are not modified.
+- Feature arrays in the parent **Attribute Matrix** of *Number of Neighbors* are compacted to remove inactive feature tuples.
+- Feature `NeighborList` arrays are removed because their contents become invalid after the topology changes.
+
+## Validation and Failure Conditions
+
+- Preflight fails with `-55571` when *Cell Feature Ids* does not contain exactly one tuple per **Image Geometry** cell.
+- Preflight fails with `-252` when *Number of Neighbors* and the enabled *Feature Phases* array do not have matching tuple counts.
+- Execution fails with `-5555` when the selected *Phase Index* is not present in *Feature Phases*.
+- Execution fails with `-55569` when the selected minimum would reject every eligible non-background **Feature**.
+- Execution fails with `-55567` when a non-negative Feature ID is outside the feature tuple range.
+- Execution fails with `-55572` when unresolved cells remain but none has a non-negative face neighbor. This prevents the non-terminating coarsening behavior present in DREAM3D 6.5.171. Ensure that every negative or rejected region is face-connected to at least one retained **Feature**.
+
+Setting *Minimum Number Neighbors* to *0* retains all existing feature tuples, but negative Feature IDs may still be reassigned.
 
 ## WARNING: Feature Data Will Become Invalid
 
-By modifying the cell level data, any feature data that was previously computed will most likely be invalid at this point. Filters that compute feature level data should be rerun to ensure accurate final results from your pipeline.
+Modifying Feature IDs changes the feature topology. Previously computed feature-level data may therefore be invalid even after its tuples are compacted. Rerun filters that compute feature data after this **Filter**.
 
 ## WARNING: NeighborList Removal
 
-If the Cell Feature AttributeMatrix contains any *NeighborList* data arrays, those arrays will be **REMOVED** because those lists are now invalid. Re-run the *Find Neighbors* filter to re-create the lists.
+If the feature **Attribute Matrix** contains any `NeighborList` arrays, those arrays are **REMOVED** because the lists are no longer valid. Rerun the [Compute Feature Neighbors](./ComputeFeatureNeighborsFilter.md) **Filter** to recreate them.
 
 % Auto generated parameter table will be inserted here
 
+## Typical Workflow
+
+1. Run the [Compute Feature Neighbors](./ComputeFeatureNeighborsFilter.md) **Filter** to create the *Number of Neighbors* array.
+2. Run this **Filter** with a conservative minimum that leaves at least one retained **Feature** connected to every rejected region.
+3. Rerun the [Compute Feature Neighbors](./ComputeFeatureNeighborsFilter.md) **Filter** and any other filters that calculate feature-level data.
+
 ## Example Pipelines
 
-+ (02) Small IN100 Full Reconstruction
+- `(02) Small IN100 Full Reconstruction`
+
+## Differences from DREAM3D 6.5.171
+
+SIMPLNX safely handles three malformed or unresolved Feature ID conditions that can cause invalid memory access or non-termination in DREAM3D 6.5.171:
+
+- [D1: Negative Feature IDs with a valid face neighbor](../vv/deviations/RequireMinNumNeighborsFilter.md#requireminnumneighborsfilter-d1)
+- [D2: Non-negative Feature IDs outside the feature tuple range](../vv/deviations/RequireMinNumNeighborsFilter.md#requireminnumneighborsfilter-d2)
+- [D3: Coarsening cannot make progress](../vv/deviations/RequireMinNumNeighborsFilter.md#requireminnumneighborsfilter-d3)
+
+For valid, non-negative, in-range Feature IDs that can be fully coarsened, the verified SIMPLNX and DREAM3D 6.5.171 outputs match.
+
+## Related Filters
 
+- [Compute Feature Neighbors](./ComputeFeatureNeighborsFilter.md)
 
 ## License & Copyright
 

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/RequireMinNumNeighbors.cpp

@@ -10,30 +10,68 @@ using namespace nx::core;
 
 namespace
 {
+constexpr int32 k_NoCoarseningProgress = -55572;
+constexpr int32 k_CopyDestinationTupleOutOfRange = -55568;
+constexpr int32 k_CopySourceTupleOutOfRange = -55573;
+
+std::string ConvertDataPathsToString(const std::vector<DataPath>& paths)
+{
+  std::stringstream ss;
+  ss << "[";
+  for(usize i = 0; i < paths.size(); i++)
+  {
+    if(i != 0)
+    {
+      ss << ", ";
+    }
+    ss << paths[i].toString();
+  }
+  ss << "]";
+  return ss.str();
+}
+
 Result<> CopyTupleFromArray(DataStructure& dataStructure, const DataPath& dataArrayPath, const std::vector<usize>& badFeatureIdIndexes, const AbstractDataStore<int32_t>& featureIds,
                             const std::vector<int32>& neighbors, const IFilter::MessageHandler& mesgHandler)
 {
-  auto* voxelArray = dataStructure.getDataAs<IDataArray>(dataArrayPath);
-  auto arraySize = voxelArray->getSize();
-  for(const auto& featureIdIndex : badFeatureIdIndexes)
+  auto& voxelArray = dataStructure.getDataRefAs<IDataArray>(dataArrayPath);
+  const usize tupleCount = voxelArray.getNumberOfTuples();
+  const usize featureIdTupleCount = featureIds.getNumberOfTuples();
+  const usize neighborMapSize = neighbors.size();
+
+  for(const usize featureIdIndex : badFeatureIdIndexes)
   {
+    // Validate the destination before indexing featureIds or neighbors.
+    if(featureIdIndex >= tupleCount || featureIdIndex >= featureIdTupleCount || featureIdIndex >= neighborMapSize)
+    {
+      const std::string message =
+          fmt::format("Cannot copy into tuple index {} of array '{}'. The array contains {} tuples, the Feature Ids array contains {} tuples, and the neighbor map contains {} entries.",
+                      featureIdIndex, dataArrayPath.toString(), tupleCount, featureIdTupleCount, neighborMapSize);
+      mesgHandler(IFilter::Message{IFilter::Message::Type::Info, message});
+      return MakeErrorResult(k_CopyDestinationTupleOutOfRange, message);
+    }
+
     const int32 featureName = featureIds.getValue(featureIdIndex);
     const int32 neighbor = neighbors[featureIdIndex];
 
-    if((neighbor >= arraySize || featureIdIndex >= arraySize) && (featureName < 0 && neighbor >= 0 && featureIds.getValue(neighbor) >= 0))
+    if(featureName >= 0 || neighbor < 0)
     {
-      const std::string message =
-          fmt::format("Out of range: While trying to copy a tuple from index {} to index {}\n  Array Name: {}\n  Num. Tuples: {}", neighbor, featureIdIndex, dataArrayPath.toString(), arraySize);
-      mesgHandler(nx::core::IFilter::Message{nx::core::IFilter::Message::Type::Info, message});
-      return MakeErrorResult(-55568, message);
+      continue;
     }
 
-    if(featureName < 0 && neighbor >= 0)
+    const usize neighborIndex = static_cast<usize>(neighbor);
+
+    // Validate the source before reading it or passing it to copyTuple().
+    if(neighborIndex >= tupleCount || neighborIndex >= featureIdTupleCount)
     {
-      if(const int32 fId = featureIds.getValue(neighbor); fId >= 0)
-      {
-        voxelArray->copyTuple(neighbor, featureIdIndex);
-      }
+      const std::string message = fmt::format("Cannot copy from tuple index {} to tuple index {} of array '{}'. The array contains {} tuples and the Feature Ids array contains {} tuples.",
+                                              neighborIndex, featureIdIndex, dataArrayPath.toString(), tupleCount, featureIdTupleCount);
+      mesgHandler(IFilter::Message{IFilter::Message::Type::Info, message});
+      return MakeErrorResult(k_CopySourceTupleOutOfRange, message);
+    }
+
+    if(featureIds.getValue(neighborIndex) >= 0)
+    {
+      voxelArray.copyTuple(neighborIndex, featureIdIndex);
     }
   }
   return {};
@@ -76,6 +114,16 @@ Result<> RequireMinNumNeighbors::operator()()
   }
   std::vector<DataPath> cellDataArrayPaths = result.value();
 
+  // FeatureIds controls whether a voxel has been reassigned, so it must always be
+  // updated even if it was included in the ignored-array selection. Keep it last
+  // so the other cell arrays read the original FeatureIds during tuple copying.
+  auto featureIdsIter = std::find(cellDataArrayPaths.begin(), cellDataArrayPaths.end(), m_InputValues->FeatureIdsPath);
+  if(featureIdsIter != cellDataArrayPaths.end())
+  {
+    cellDataArrayPaths.erase(featureIdsIter);
+  }
+  cellDataArrayPaths.push_back(m_InputValues->FeatureIdsPath);
+
   // Run the algorithm.
   // This was checked up in the execute function (which is called before this function),
   // so if we got this far then all should be good with the return. We might get
@@ -148,7 +196,19 @@ Result<> RequireMinNumNeighbors::operator()()
   // Mark all features to be removed with a -1 value.
   for(usize i = 0; i < totalPoints; i++)
   {
-    int32 featureId = featureIds[i];
+    const int32 featureId = featureIds[i];
+
+    if(featureId < 0)
+    {
+      continue;
+    }
+
+    if(static_cast<usize>(featureId) >= totalFeatures)
+    {
+      return MakeErrorResult(-55567, fmt::format("Feature ID '{}' in array '{}' is outside the valid range [0, {}). '{}' MAY HAVE BEEN MODIFIED.", featureId, m_InputValues->FeatureIdsPath.toString(),
+                                                 totalFeatures, ConvertDataPathsToString(cellDataArrayPaths)));
+    }
+
     if(!activeObjects[featureId])
     {
       featureIds[i] = -1;
@@ -168,7 +228,6 @@ Result<> RequireMinNumNeighbors::operator()()
   int32 current = 0;
   int32 most = 0;
   int64 neighborPoint = 0;
-  usize numFeatures = numNeighbors.getNumberOfTuples();
 
   constexpr FaceNeighborType k_NumFaceNeighbors = VoxelNeighbors<Image3D>::k_FaceNeighborCount;
   const std::array<int64, k_NumFaceNeighbors> neighborVoxelIndexOffsets = initializeFaceNeighborOffsets(dims);
@@ -180,8 +239,7 @@ Result<> RequireMinNumNeighbors::operator()()
   int64 jStride = 0;
   int32 featureName = 0;
   int32 feature = 0;
-  int32 neighbor = 0;
-  std::vector<int32> voteCount(numFeatures + 1, 0);
+  std::vector<int32> voteCount(totalFeatures + 1, 0);
   std::vector<usize> badFeatureIdIndexes;
 
   while(counter != 0)
@@ -191,6 +249,7 @@ Result<> RequireMinNumNeighbors::operator()()
       return {};
     }
     counter = 0;
+    bool madeFill = false;
     badFeatureIdIndexes.clear();
     for(int64 zIdx = 0; zIdx < dims[2]; zIdx++)
     {
@@ -227,19 +286,27 @@ Result<> RequireMinNumNeighbors::operator()()
                   {
                     most = current;
                     neighbors[voxelIndex] = neighborPoint;
+                    madeFill = true;
                   }
                 }
               }
             }
 
-            // Reset the voteCount array to all zeros
-            std::fill(voteCount.begin(), voteCount.end(), 0);
-          }
-          else if(featureName >= numFeatures)
-          {
-            std::string message = fmt::format("Error: Found a feature Id '{}' that is >= the number of features '{}' at voxel index X={},Y={},Z={}.", featureName, numFeatures, xIdx, yIdx, zIdx);
-            m_MessageHandler(nx::core::IFilter::Message{nx::core::IFilter::Message::Type::Info, message});
-            return MakeErrorResult(-55567, message);
+            // Reset the vote counts touched by this voxel
+            for(const auto& faceIndex : faceNeighborInternalIdx)
+            {
+              if(!isValidFaceNeighbor[faceIndex])
+              {
+                continue;
+              }
+
+              const int64 resetNeighborPoint = voxelIndex + neighborVoxelIndexOffsets[faceIndex];
+              const int32 resetFeature = featureIds[resetNeighborPoint];
+              if(resetFeature >= 0)
+              {
+                voteCount[resetFeature] = 0;
+              }
+            }
           }
         }
       }
@@ -248,21 +315,28 @@ Result<> RequireMinNumNeighbors::operator()()
     std::string message = fmt::format("{} voxels to update..", counter);
     m_MessageHandler(nx::core::IFilter::Message::Type::Info, message);
 
-    // TODO: This can be parallelized much like NeighborOrientationCorrelation, just do not update the featureIds array during that section. Wait until everything is complete
-    // This next section finds the "FeatureIds" array and moves that array to the end of the list
-    auto featureIdsIter = std::find(cellDataArrayPaths.begin(), cellDataArrayPaths.end(), m_InputValues->FeatureIdsPath);
-    if(featureIdsIter != cellDataArrayPaths.end())
+    if(counter != 0 && !madeFill)
     {
-      cellDataArrayPaths.erase(featureIdsIter);
-      cellDataArrayPaths.push_back(m_InputValues->FeatureIdsPath);
+      return MakeErrorResult(
+          k_NoCoarseningProgress,
+          fmt::format("Unable to reassign {} cell(s) in Feature Ids array '{}' because none has a non-negative face neighbor. Ensure the array contains at least one cell assigned to a feature "
+                      "that meets the minimum-neighbor requirement. THE FOLLOWING ARRAYS MAY HAVE BEEN MODIFIED: '{}'",
+                      counter, m_InputValues->FeatureIdsPath.toString(), ConvertDataPathsToString(cellDataArrayPaths)));
     }
+
+    // TODO: This can be parallelized much like NeighborOrientationCorrelation, just do not update the featureIds array during that section. Wait until everything is complete
     for(const auto& cellArrayPath : cellDataArrayPaths)
     {
       if(m_ShouldCancel)
       {
         return {};
       }
-      CopyTupleFromArray(m_DataStructure, cellArrayPath, badFeatureIdIndexes, featureIds, neighbors, m_MessageHandler);
+      auto copyResult = CopyTupleFromArray(m_DataStructure, cellArrayPath, badFeatureIdIndexes, featureIds, neighbors, m_MessageHandler);
+      if(copyResult.invalid())
+      {
+        copyResult.warnings().push_back({-55574, fmt::format("THE FOLLOWING ARRAYS MAY HAVE BEEN MODIFIED: '{}'", ConvertDataPathsToString(cellDataArrayPaths))});
+        return copyResult;
+      }
     }
   }
 
@@ -277,7 +351,12 @@ Result<> RequireMinNumNeighbors::operator()()
 
   m_MessageHandler(IFilter::Message::Type::Info, fmt::format("Feature Count Changed: Previous: {} New: {}", totalFeatures, count));
   DataPath cellFeatureGroupPath = m_InputValues->NumNeighborsPath.getParent();
-  nx::core::RemoveInactiveObjects(m_DataStructure, cellFeatureGroupPath, activeObjects, featureIds, totalFeatures, m_MessageHandler, m_ShouldCancel);
+  if(!nx::core::RemoveInactiveObjects(m_DataStructure, cellFeatureGroupPath, activeObjects, featureIds, totalFeatures, m_MessageHandler, m_ShouldCancel))
+  {
+    return MakeErrorResult(
+        -55570, fmt::format("Failed to remove inactive feature tuples from feature group '{}'. Check that its arrays match the tuple count of '{}'. THE FOLLOWING ARRAYS MAY HAVE BEEN MODIFIED: '{}'",
+                            cellFeatureGroupPath.toString(), m_InputValues->NumNeighborsPath.toString(), ConvertDataPathsToString(cellDataArrayPaths)));
+  }
 
   return {};
 }

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/RequireMinNumNeighborsFilter.cpp

@@ -19,6 +19,7 @@ namespace nx::core
 namespace
 {
 constexpr int32 k_InconsistentTupleCount = -252;
+constexpr int32 k_FeatureIdsTupleCountMismatch = -55571;
 
 } // namespace
 
@@ -119,10 +120,15 @@ IFilter::PreflightResult RequireMinNumNeighborsFilter::preflightImpl(const DataS
 
   std::vector<DataPath> dataArrayPaths;
 
-  ShapeType cDims = {1};
   auto& featureIds = dataStructure.getDataRefAs<Int32Array>(featureIdsPath);
+  auto& imageGeom = dataStructure.getDataRefAs<ImageGeom>(imageGeomPath);
+  if(featureIds.getNumberOfTuples() != imageGeom.getNumberOfCells())
+  {
+    return MakePreflightErrorResult(k_FeatureIdsTupleCountMismatch,
+                                    fmt::format("FeatureIds array '{}' contains {} tuples, but Image Geometry '{}' contains {} cells. Select a FeatureIds array that matches the Image Geometry.",
+                                                featureIdsPath.toString(), featureIds.getNumberOfTuples(), imageGeomPath.toString(), imageGeom.getNumberOfCells()));
+  }
 
-  auto& numNeighborsArray = dataStructure.getDataRefAs<Int32Array>(numNeighborsPath);
   dataArrayPaths.push_back(numNeighborsPath);
 
   if(applyToSinglePhase)