From 68bec0a0194d58973f9800eb8d79241091095603 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Fri, 5 Jun 2026 08:38:59 -0400 Subject: [PATCH 1/9] Remove Legacy Test --- .../test/ComputeFeatureSizesTest.cpp | 72 ------------------- 1 file changed, 72 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index 4348552717..6c915c4140 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -8,18 +8,11 @@ #include #include -#include using namespace nx::core; namespace fs = std::filesystem; -namespace LegacyTest -{ -const std::string k_Volumes("Volumes"); -const std::string k_EquivalentDiameters("EquivalentDiameters"); -} // namespace LegacyTest - namespace Test { // Geometry Level @@ -647,71 +640,6 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: Invalid: Preflight Failure", "[Simp #endif } -TEST_CASE("SimplnxCore::ComputeFeatureSizes: Legacy: Small IN100 Test", "[SimplnxCore][ComputeFeatureSizes]") -{ - UnitTest::LoadPlugins(); - - const nx::core::UnitTest::TestFileSentinel testDataSentinel(nx::core::unit_test::k_TestFilesDir, "6_6_stats_test_v2.tar.gz", "6_6_stats_test_v2.dream3d"); - - // Read the Small IN100 Data set - auto baseDataFilePath = fs::path(fmt::format("{}/6_6_stats_test_v2.dream3d", unit_test::k_TestFilesDir)); - DataStructure dataStructure = UnitTest::LoadDataStructure(baseDataFilePath); - DataPath smallIn100Group({Constants::k_DataContainer}); - DataPath cellDataPath = smallIn100Group.createChildPath(Constants::k_CellData); - DataPath cellPhasesPath = cellDataPath.createChildPath(Constants::k_Phases); - DataPath featureIdsPath = cellDataPath.createChildPath(Constants::k_FeatureIds); - DataPath featureGroup = smallIn100Group.createChildPath(Constants::k_CellFeatureData); - std::string volumesName = "computed_volumes"; - std::string numElementsName = "computed_NumElements"; - std::string EquivalentDiametersName = "computed_EquivalentDiameters"; - - std::vector featureNames = {LegacyTest::k_Volumes, LegacyTest::k_EquivalentDiameters, Constants::k_NumElements}; - - { - ComputeFeatureSizesFilter filter; - Arguments args; - - args.insert(ComputeFeatureSizesFilter::k_GeometryPath_Key, std::make_any(smallIn100Group)); - args.insert(ComputeFeatureSizesFilter::k_SaveElementSizes_Key, std::make_any(false)); - args.insert(ComputeFeatureSizesFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(featureIdsPath)); - args.insert(ComputeFeatureSizesFilter::k_CellFeatureAttributeMatrixPath_Key, std::make_any(featureGroup)); - args.insert(ComputeFeatureSizesFilter::k_VolumesName_Key, std::make_any(volumesName)); - args.insert(ComputeFeatureSizesFilter::k_EquivalentDiametersName_Key, std::make_any(EquivalentDiametersName)); - args.insert(ComputeFeatureSizesFilter::k_NumElementsName_Key, std::make_any(numElementsName)); - - // 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); - } - - // Compare Outputs - { - DataPath exemplaryDataPath = featureGroup.createChildPath(LegacyTest::k_Volumes); - UnitTest::CompareArrays(dataStructure, exemplaryDataPath, featureGroup.createChildPath(volumesName)); - } - - { - DataPath exemplaryDataPath = featureGroup.createChildPath(LegacyTest::k_EquivalentDiameters); - UnitTest::CompareArrays(dataStructure, exemplaryDataPath, featureGroup.createChildPath(EquivalentDiametersName)); - } - - { - DataPath exemplaryDataPath = featureGroup.createChildPath(Constants::k_NumElements); - UnitTest::CompareArrays(dataStructure, exemplaryDataPath, featureGroup.createChildPath(numElementsName)); - } - -// Write the DataStructure out to the file system -#ifdef SIMPLNX_WRITE_TEST_OUTPUT - WriteTestDataStructure(dataStructure, fs::path(fmt::format("{}/calculate_feature_sizes/legacy_test.dream3d", unit_test::k_BinaryTestOutputDir))); -#endif - - UnitTest::CheckArraysInheritTupleDims(dataStructure); -} - TEST_CASE("SimplnxCore::ComputeFeatureSizesFilter: SIMPL Backwards Compatibility", "[SimplnxCore][ComputeFeatureSizesFilter][BackwardsCompatibility]") { auto app = Application::GetOrCreateInstance(); From 7c7c9cd2b50b1a540d8ea9c745d5973f46667885 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Fri, 5 Jun 2026 13:05:14 -0400 Subject: [PATCH 2/9] provenance file --- .../test/ComputeFeatureSizesTest.cpp | 2 +- .../provenance/ComputeFeatureSizesFilter.md | 426 ++++++++++++++++++ 2 files changed, 427 insertions(+), 1 deletion(-) create mode 100644 src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index 6c915c4140..27933bbfe3 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -64,7 +64,7 @@ DataStructure Create2DImageDataStructure() 1, 1, 1, 1, 1, 1, 1, 1, 3, 3, 3, 3, 1, 1, 3, - 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3 }; // clang-format on diff --git a/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md new file mode 100644 index 0000000000..d1466a0898 --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md @@ -0,0 +1,426 @@ +# Exemplar Archive Provenance: Inlined + +This sidecar records how an exemplar archive used in unit tests was generated. It is the answer to "where did this gold-standard data come from?" + +One sidecar per archive. The archive name and SHA512 must match `download_test_data()` in `src/Plugins/

/test/CMakeLists.txt`. + +--- + +## Archive identity + +| Field | Value | +|---|---| +| **Archive** | `Inlined - CreateRectGridDataStructure()` | +| **SHA512** | `N/A` | +| **Used by tests** | `Valid: Rectilinear Grid`, `Valid: Rectilinear Grid with Element Size` | +| **Generated by** | *Nathan Young* | +| **Generated on** | *2026-02-20* | +| **Generated at commit** | `34c86ab4e419725728ddb6270119bb40ea4c2945` | + +## How it was generated + +The `CreateRectGridDataStructure()` function generates a 4x4x4 Rectilinear Grid Geometry and all descendant arrays on each test invocation. The actual inlined data in the function was synthesized by hand to test typical functionality. The expected output is a Class 1 Oracle as it is hand derived. + +Relevant Information for the data set: + +```text +Image Info: +Dimensions: {4, 4, 4} +xBounds: {0.0f, 0.6f, 0.9f, 2.1f, 13.0f} +yBounds: {0.0f, 0.1f, 1.0f, 10.0f, 100.0f} +zBounds: {0.0f, 1.0f, 1.2f, 2.0f, 2.1f} + +Feature Ids: +1, 2, 2, 2, +1, 1, 1, 1, +1, 1, 1, 2, +2, 2, 1, 1, + +1, 1, 1, 1, +1, 1, 1, 1, +1, 1, 1, 1, +1, 1, 1, 1, + +3, 3, 3, 3, +1, 3, 1, 3, +2, 2, 1, 1, +2, 2, 1, 1, + +3, 2, 2, 1, +3, 2, 1, 1, +3, 1, 1, 1, +3, 2, 1, 2 +``` + +## Canonical oracle output + +| DataPath | Source of expected values | +|---|---| +| `Image/FeatureData/NumElements` | *Class 1 hand derivation* | +| `Image/FeatureData/Volumes` | *Class 1 hand derivation* | +| `Image/FeatureData/EquivalentDiameters` | *Class 1 hand derivation* | + +The expected outputs are as follows: + +```text +NumElements: {0, 39, 15, 10} +Volumes: {0.0, 2358.834, 352.462, 15.104} +EquivalentDiameters: {0.0, 16.516, 8.764, 3.0994} +``` + +In order to derive these values, the voxels must be processed individually and summed up. This is how it was done: + +```text +======================== +| Totals By Feature | +======================== +| IDs |Count| Volume | +| 1 | 39 | 2362.434 | +| 2 | 15 | 352.462 | +| 3 | 10 | 15.104 | +======================== +``` + +Format + +```text +:::::::::::::::::::::::::: +X Y Z | Volume | Feature | +:::::::::::::::::::::::::: <- Row Divider + <- Plane Divider +:::::::::::::::::::::::::: +``` + +Geometry Structure Information + +xBounds -> 0.0f, 0.6f, 0.9f, 2.1f, 13.0f +yBounds -> 0.0f, 0.1f, 1.0f, 10.0f, 100.0f +zBounds -> 0.0f, 1.0f, 1.2f, 2.0f, 2.1f + +Data + +```text +:::::::::::::::::::::::: +0.6 0.1 1.0 |0.06 | 1 | 0.06 +0.3 0.1 1.0 |0.03 | 2 | 0.03 +1.2 0.1 1.0 |0.12 | 2 | 0.15 +10.9 0.1 1.0 |1.09 | 2 | 1.24 +:::::::::::::::::::::::: +0.6 0.9 1.0 |0.54 | 1 | 0.60 +0.3 0.9 1.0 |0.27 | 1 | 0.87 +1.2 0.9 1.0 |1.08 | 1 | 1.95 +10.9 0.9 1.0 |9.81 | 1 | 11.76 +:::::::::::::::::::::::: +0.6 9.0 1.0 |5.4 | 1 | 17.16 +0.3 9.0 1.0 |2.7 | 1 | 19.86 +1.2 9.0 1.0 |10.8 | 1 | 30.66 +10.9 9.0 1.0 |98.1 | 2 | 99.34 +:::::::::::::::::::::::: +0.6 90.0 1.0 |54 | 2 | 153.34 +0.3 90.0 1.0 |27 | 2 | 180.34 +1.2 90.0 1.0 |108 | 1 | 138.66 +10.9 90.0 1.0|981 | 1 | 1119.66 +:::::::::::::::::::::::: + +======================== +| Slice 1 By Feature | +======================== +| IDs |Count| Volume | +| 1 | 10 | 1119.66 | +| 2 | 6 | 180.34 | +| 3 | 0 | 0.0 | +======================== + +:::::::::::::::::::::::: +0.6 0.1 0.2 |0.012| 1 | 0.012 +0.3 0.1 0.2 |0.006| 1 | 0.018 +1.2 0.1 0.2 |0.024| 1 | 0.042 +10.9 0.1 0.2 |0.218| 1 | 0.260 +:::::::::::::::::::::::: +0.6 0.9 0.2 |0.108| 1 | 0.368 +0.3 0.9 0.2 |0.054| 1 | 0.422 +1.2 0.9 0.2 |0.216| 1 | 0.638 +10.9 0.9 0.2 |1.962| 1 | 2.600 +:::::::::::::::::::::::: +0.6 9.0 0.2 |1.08 | 1 | 3.680 +0.3 9.0 0.2 |0.54 | 1 | 4.220 +1.2 9.0 0.2 |2.16 | 1 | 6.380 +10.9 9.0 0.2 |19.62| 1 | 26.00 +:::::::::::::::::::::::: +0.6 90.0 0.2 |10.8 | 1 | 36.80 +0.3 90.0 0.2 |5.4 | 1 | 42.20 +1.2 90.0 0.2 |21.6 | 1 | 63.80 +10.9 90.0 0.2|196.2| 1 | 260.0 +:::::::::::::::::::::::: + +======================== +| Slice 2 By Feature | +======================== +| IDs |Count| Volume | +| 1 | 16 | 260 | +| 2 | 0 | 0.0 | +| 3 | 0 | 0.0 | +======================== + +:::::::::::::::::::::::: +0.6 0.1 0.8 |0.048| 3 | 0.048 +0.3 0.1 0.8 |0.024| 3 | 0.072 +1.2 0.1 0.8 |0.096| 3 | 0.168 +10.9 0.1 0.8 |0.872| 3 | 1.04 +:::::::::::::::::::::::: +0.6 0.9 0.8 |0.432| 1 | 0.432 +0.3 0.9 0.8 |0.216| 3 | 1.256 +1.2 0.9 0.8 |0.864| 1 | 1.296 +10.9 0.9 0.8 |7.848| 3 | 9.104 +:::::::::::::::::::::::: +0.6 9.0 0.8 |4.32 | 2 | 4.320 +0.3 9.0 0.8 |2.16 | 2 | 6.480 +1.2 9.0 0.8 |8.64 | 1 | 9.936 +10.9 9.0 0.8 |78.48| 1 | 88.416 +:::::::::::::::::::::::: +0.6 90.0 0.8 |43.2 | 2 | 49.68 +0.3 90.0 0.8 |21.6 | 2 | 71.28 +1.2 90.0 0.8 |86.4 | 1 | 174.816 +10.9 90.0 0.8|784.8| 1 | 959.616 +:::::::::::::::::::::::: + +======================== +| Slice 3 By Feature | +======================== +| IDs |Count| Volume | +| 1 | 6 | 959.616 | +| 2 | 4 | 71.28 | +| 3 | 6 | 9.104 | +======================== + +:::::::::::::::::::::::: +0.6 0.1 0.1 |0.006| 3 | 0.006 +0.3 0.1 0.1 |0.003| 2 | 0.003 +1.2 0.1 0.1 |0.012| 2 | 0.015 +10.9 0.1 0.1 |0.109| 1 | 0.109 +:::::::::::::::::::::::: +0.6 0.9 0.1 |0.054| 3 | 0.060 +0.3 0.9 0.1 |0.027| 2 | 0.042 +1.2 0.9 0.1 |0.108| 1 | 0.217 +10.9 0.9 0.1 |0.981| 1 | 1.198 +:::::::::::::::::::::::: +0.6 9.0 0.1 |0.54 | 3 | 0.60 +0.3 9.0 0.1 |0.27 | 1 | 1.468 +1.2 9.0 0.1 |1.08 | 1 | 2.548 +10.9 9.0 0.1 |9.81 | 1 | 12.358 +:::::::::::::::::::::::: +0.6 90.0 0.1 |5.4 | 3 | 6.0 +0.3 90.0 0.1 |2.7 | 2 | 2.742 +1.2 90.0 0.1 |10.8 | 1 | 23.158 +10.9 90.0 0.1|98.1 | 2 | 100.842 +:::::::::::::::::::::::: + +======================== +| Slice 4 By Feature | +======================== +| IDs |Count| Volume | +| 1 | 7 | 23.158 | +| 2 | 5 | 100.842 | +| 3 | 4 | 6.0 | +======================== +``` + +```text +Calculated Output + + 0.06 0.03 0.12 1.09 + 0.54 0.27 1.08 9.81 + 5.4 2.7 10.8 98.1 + 54 27 108 981 + + 0.012 0.006 0.024 0.218 + 0.108 0.054 0.216 1.962 + 1.08 0.54 2.16 19.62 + 10.8 5.4 21.6 196.2 + + 0.048 0.024 0.096 0.872 + 0.432 0.216 0.864 7.848 + 4.32 2.16 8.64 78.48 + 43.2 21.6 86.4 784.8 + + 0.00599999 0.003 0.012 0.109 + 0.0539999 0.027 0.108 0.980999 + 0.539999 0.27 1.08 9.80999 + 5.39999 2.7 10.8 98.0999 + ``` + +We calculate the `EquivalentDiameters` with the volumes: + +Given that volume of a circle can be derived by `Volume = 4/3 * pi * r^3`, we can isolate the radius given volume. That formula becomes `radius = cubed_root((3 * Volume) / (4 * pi))`, and given that the radius is half of the diameter the formula becomes `Diameter = 2 * cubed_root((3 * Volume) / (4 * pi))`. + +## Oracle provenance (Classes 2, 3, 5 only) + +N/A + +## Second-engineer oracle review + +- **Reviewer:** ** OR *skipped* +- **Date:** *YYYY-MM-DD* +- **Skip reason** (if skipped): ** + +## Regenerated to fix a circular-oracle situation? + +N/A + +--- + +## Archive identity + +| Field | Value | +|---|---| +| **Archive** | `Inlined - Create2DImageDataStructure()` | +| **SHA512** | `N/A` | +| **Used by tests** | `Valid: Image 2D`, `Valid: Image 2D with Element Sizes` | +| **Generated by** | *Nathan Young* | +| **Generated on** | *2026-02-20* | +| **Generated at commit** | `34c86ab4e419725728ddb6270119bb40ea4c2945` | + +## How it was generated + +The `Create2DImageDataStructure()` function generates a 5x5x1 Image Geometry and all descendant arrays on each test invocation. The actual inlined data in the function was synthesized by hand to test typical functionality as well as an isolated voxel edge case. The expected output is a Class 1 Oracle as it is hand derived. + +Relevant Information for the data set: + +```text +Image Info: +Dimensions: {5,5,1} +Spacing: {20.2f, 0.1f, 1.0f} +Origin: {0.0f, 0.0f, 0.0f} + +Feature Ids: +1, 2, 3, 3, 3, +1, 1, 1, 1, 1, +1, 1, 1, 3, 3, +3, 3, 1, 1, 3, +3, 3, 3, 3, 3 +``` + +## Canonical oracle output + +| DataPath | Source of expected values | +|---|---| +| `Image/FeatureData/NumElements` | *Class 1 hand derivation* | +| `Image/FeatureData/Volumes` | *Class 1 hand derivation* | +| `Image/FeatureData/EquivalentDiameters` | *Class 1 hand derivation* | + +The expected outputs are as follows: + +```text +NumElements: {0, 11, 1, 13} +Areas: {0.0, 22.22, 2.02, 26.26} +EquivalentDiameters: {0.0, 5.319, 1.6037, 5.782} +``` + +In order to derive these values, one first calculates single voxel area which in this case is `2.02`. Then feature counts must be determined using the *Feature Ids* array, this gives the `NumElements` results. With the feature counts, we then determine the size of the feature via multiplying the number of elements in each feature by the voxel area, which gives the `Areas`. Finally, we calculate the `EquivalentDiameters` with the areas: + +Given that area of a circle can be derived by `Area = pi * radius^2`, we can isolate the radius given area. That formula becomes `radius = square_root(Area / pi)`, and given that the radius is half of the diameter the formula becomes `Diameter = 2 * square_root(A / pi)`. + +## Oracle provenance (Classes 2, 3, 5 only) + +N/A + +## Second-engineer oracle review + +- **Reviewer:** ** OR *skipped* +- **Date:** *YYYY-MM-DD* +- **Skip reason** (if skipped): ** + +## Regenerated to fix a circular-oracle situation? + +N/A + +--- + +## Archive identity + +| Field | Value | +|---|---| +| **Archive** | `Inlined - Create3DImageDataStructure()` | +| **SHA512** | `N/A` | +| **Used by tests** | `Valid: Image 3D`, `Valid: Image 3D with Element Sizes` | +| **Generated by** | *Nathan Young* | +| **Generated on** | *2026-02-20* | +| **Generated at commit** | `34c86ab4e419725728ddb6270119bb40ea4c2945` | + +## How it was generated + +The `Create3DImageDataStructure()` function generates a 5x5x5 Image Geometry and all descendant arrays on each test invocation. The actual inlined data in the function was synthesized by hand to test typical functionality. The expected output is a Class 1 Oracle as it is hand derived. + +Relevant Information for the data set: + +```text +Image Info: +Dimensions: {5,5,5} +Spacing: {1.2f, 0.9f, 2.1f} +Origin: {0.0f, 0.0f, 0.0f} + +Feature Ids: +1, 2, 2, 2, 2, +1, 1, 1, 1, 1, +1, 1, 1, 2, 2, +2, 2, 1, 1, 2, +2, 2, 2, 2, 2, + +1, 1, 1, 1, 1, +1, 1, 1, 1, 1, +1, 1, 1, 1, 1, +1, 1, 1, 1, 1, +1, 1, 1, 1, 1, + +3, 3, 3, 3, 1, +1, 3, 1, 3, 3, +2, 2, 1, 1, 1, +2, 2, 1, 1, 3, +2, 2, 1, 3, 3, + +3, 2, 2, 1, 1, +3, 2, 1, 1, 1, +3, 1, 1, 1, 1, +3, 2, 1, 2, 1, +3, 2, 2, 2, 2, + +3, 1, 3, 1, 1, +1, 1, 1, 1, 1, +3, 1, 1, 1, 3, +1, 1, 1, 3, 1, +3, 1, 3, 1, 3 +``` + +## Canonical oracle output + +| DataPath | Source of expected values | +|---|---| +| `Image/FeatureData/NumElements` | *Class 1 hand derivation* | +| `Image/FeatureData/Volumes` | *Class 1 hand derivation* | +| `Image/FeatureData/EquivalentDiameters` | *Class 1 hand derivation* | + +The expected outputs are as follows: + +```text +NumElements: {0, 73, 29, 23} +Volumes: {0.0, 165.564, 65.772, 52.164} +EquivalentDiameters: {0.0, 6.813, 5.008, 4.636} +``` + +In order to derive these values, one first calculates single voxel volume which in this case is `2.268`. Then feature counts must be determined using the *Feature Ids* array, this gives the `NumElements` results. With the feature counts, we then determine the size of the feature via multiplying the number of elements in each feature by the voxel volume, which gives the `Volumes`. Finally, we calculate the `EquivalentDiameters` with the volumes: + +Given that volume of a circle can be derived by `Volume = 4/3 * pi * r^3`, we can isolate the radius given volume. That formula becomes `radius = cubed_root((3 * Volume) / (4 * pi))`, and given that the radius is half of the diameter the formula becomes `Diameter = 2 * cubed_root((3 * Volume) / (4 * pi))`. + +## Oracle provenance (Classes 2, 3, 5 only) + +N/A + +## Second-engineer oracle review + +- **Reviewer:** ** OR *skipped* +- **Date:** *YYYY-MM-DD* +- **Skip reason** (if skipped): ** + +## Regenerated to fix a circular-oracle situation? + +N/A From 9f5c2468f3f99839fef24ea110a022627b42c5a3 Mon Sep 17 00:00:00 2001 From: nyoungbq Date: Thu, 11 Jun 2026 14:39:46 -0400 Subject: [PATCH 3/9] - patched thread kahan summation bug - Compute Feature Sizes Deviations - Compute Feature Sizes V&V report --- .../Algorithms/ComputeFeatureSizes.cpp | 6 +- .../vv/ComputeFeatureSizesFilter.md | 106 ++++++++++++++++++ .../deviations/ComputeFeatureSizesFilter.md | 47 ++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md create mode 100644 src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp index 48993310b2..565e30ee66 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp @@ -344,13 +344,13 @@ Result<> ProcessRectGridGeom(RectGridGeom& rectGridGeom, Float32AbstractDataStor // Use Kahan summation to determine overall volume // Attempt to recover low order into the value. The first instance is 0 - const float64 value = featureVolumes[featureIdx] - featureCompensators[featureIdx]; + const float64 value = localVolumes[featureIdx] - featureCompensators[featureIdx]; // low order may be lost - const float64 volSum = localVolumes[featureIdx] + value; + const float64 volSum = featureVolumes[featureIdx] + value; // recover and cache low order - featureCompensators[featureIdx] = (volSum - localVolumes[featureIdx]) - value; + featureCompensators[featureIdx] = (volSum - featureVolumes[featureIdx]) - value; // store volumes featureVolumes[featureIdx] = volSum; diff --git a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md new file mode 100644 index 0000000000..83e4670bf5 --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md @@ -0,0 +1,106 @@ +# V&V Report: ComputeFeatureSizes + +| | | +|---|---| +| Plugin | `SimplnxCore` | +| SIMPLNX UUID | `c666ee17-ca58-4969-80d0-819986c72485` | +| DREAM3D 6.5.171 equivalent | `FindSizes` — UUID `656f144c-a120-5c3b-bee5-06deab438588` | +| Verified commit | *pending* | +| Status | COMPLETE | +| Sign-off | Nathan Young, June 10th, 2026 | + +## At a glance + +| Aspect | State | +|---|---| +| Algorithm relationship | **Port** of both `FindSizes::execute()` (ImageGeom) and `FindSizes::findSizesUnstructured()` (RectGridGeom) | +| Oracle | **Class 1 (Analytical)** — all test data inlined, hand-derived | +| Code paths | **14 of 19** exercised; 5 gaps (cancel ×2, INT32_MAX overflow ×2, other-geom no-op ×1) | +| Tests | **9 TEST_CASEs** — all pass | +| External archive | None | +| Deviations | **1 active** (`ComputeFeatureSizes-D1`): Kahan vs naive summation in RectGridGeom — see deviations file | +| Open bugs | **1 open** (`Bug-1`): 2D area formula uses all 3 spacings (= volume) instead of the 2 non-flat spacings | + +## Summary + +`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. One precision deviation (`D1`) and one open bug (`Bug-1`) are documented below. + +## Algorithm Relationship + +**Port** of `FindSizes` (both ImageGeom and RectGridGeom paths). + +- **ImageGeom** (`ProcessImageGeom`): voxel count × voxel volume/area → ESD/ECD. Identical to `FindSizes::execute()`. Formulas: `ESD = 2·∛(V / (4π/3))`; `ECD = 2·√(A / π)`. +- **RectGridGeom** (`ProcessRectGridGeom`): per-cell `ΔxΔyΔz` sizes, summed per feature. Port of `FindSizes::findSizesUnstructured()`. SIMPLNX departs in precision: float32 element sizes are promoted to float64 and Kahan summation is applied at two levels (per-thread and post-reduction). See deviation `D1`. + +Port-time additions not in DREAM3D 6.5.171: +- **Parallel execution** — `ParallelDataAlgorithm` + `tbb::combinable`; legacy was serial. +- **Execute-time FeatureId bounds check** — `ValidateFeatureIdsToFeatureAttributeMatrixIndexing`; legacy accessed out-of-bounds silently. +- **Preflight dimension guard** — rejects ImageGeom with two or more dimensions equal to 1. This is because a single empty dimension converts the calculation from Volume to Area and "1D" Images would not make sense to get an Area formula. + +## Oracle + +**Class 1 (Analytical)** — all expected values are derived from first principles and asserted in `test/ComputeFeatureSizesTest.cpp`. + +| Fixture | Geometry | Derived quantity | +|---|---|---| +| `Create2DImageDataStructure()` | 5×5×1 ImageGeom, spacing 20.2×0.1×1.0 | `voxelArea = 20.2 × 0.1 × 1.0 = 2.02`; areas = count × 2.02; ECDs from circle formula | +| `Create3DImageDataStructure()` | 5×5×5 ImageGeom, spacing 1.2×0.9×2.1 | `voxelVol = 1.2 × 0.9 × 2.1 = 2.268`; volumes = count × 2.268; ESDs from sphere formula | +| `CreateRectGridDataStructure()` | 4×4×4 non-uniform RectGrid | Per-cell `ΔxΔyΔz` summed per feature; step-by-step trace in provenance sidecar | + +Negative tests use degenerate inputs (out-of-bounds FeatureId; degenerate dims) and assert an error result is returned. + +Second-engineer review pending: verify hand-derivations for all three fixtures and the tolerance choice (`std::numeric_limits::epsilon()`). + +## Code path coverage + +14 of 19 paths exercised. Source: `Algorithms/ComputeFeatureSizes.cpp`. + +| # | Path | Exercised by | +|---|---|---| +| 1 | Preflight: `emptyDimCount > 1` → error | `Invalid: Preflight Failure` (4 sub-checks) | +| 2 | Preflight: valid dims → pass | All 6 positive tests | +| 3 | Execute validate: FeatureId > numFeatures → error | `Invalid: Execution Failure` | +| 4 | Execute validate: passes → geometry dispatch | All 6 positive tests | +| 5 | Dispatch: ImageGeom → `ProcessImageGeom` | `Image 2D *`, `Image Stack 3D *` | +| 6 | Dispatch: RectGridGeom → `ProcessRectGridGeom` | `Rectilinear Grid *` | +| 7 | Dispatch: other geometry → no-op | *Not tested. `GeometrySelectionParameter` prevents this at runtime; gap acceptable.* | +| 8 | Image: any dim == 1 → area + ECD | `Image 2D *` | +| 9 | Image: all dims > 1 → volume + ESD | `Image Stack 3D *` | +| 10 | Image: voxelCount > `k_MaxVoxelCount` → error | *Not tested. Requires >2³¹ voxels in one feature; impractical. Gap acceptable.* | +| 11 | Image: `SaveElementSizes = false` | `Image 2D`, `Image Stack 3D` | +| 12 | Image: `SaveElementSizes = true` | `Image 2D with Element Sizes`, `Image Stack 3D with Element Size` | +| 13 | Image: `shouldCancel` in loop → early return | *Not tested. Cancel disregard would cause hangs in any test; low-value gap.* | +| 14 | RectGrid: `findElementSizes` → per-cell volumes available | `Rectilinear Grid *` | +| 15 | RectGrid: Kahan summation in `RectGridSummationImpl` + post-reduction | `Rectilinear Grid *` (non-uniform spacing exercises summation) | +| 16 | RectGrid: voxelCount > `k_MaxVoxelCount` → error | *Not tested. Same rationale as path 10.* | +| 17 | RectGrid: `SaveElementSizes = false` → `deleteElementSizes()` | `Rectilinear Grid` | +| 18 | RectGrid: `SaveElementSizes = true` → element sizes retained | `Rectilinear Grid with Element Size` | +| 19 | RectGrid: `shouldCancel` in loop → early return | *Not tested. Same rationale as path 13.* | + +## Test inventory + +| Test case | Notes | +|---|---| +| `Valid: Image 2D` | Class 1; 5×5×1, spacing 20.2×0.1×1.0; SaveElementSizes=false | +| `Valid: Image 2D with Element Sizes` | Same fixture; SaveElementSizes=true | +| `Valid: Image Stack 3D` | Class 1; 5×5×5, spacing 1.2×0.9×2.1; SaveElementSizes=false | +| `Valid: Image Stack 3D with Element Size` | Same fixture; SaveElementSizes=true | +| `Valid: Rectilinear Grid` | Class 1; 4×4×4 non-uniform; SaveElementSizes=false; Kahan path exercised | +| `Valid: Rectilinear Grid with Element Size` | Same fixture; SaveElementSizes=true | +| `Invalid: Execution Failure` | FeatureId 10 in a 4-feature AM; asserts execute result invalid | +| `Invalid: Preflight Failure` | 4 degenerate dim configs; asserts preflight result invalid | +| `SIMPL Backwards Compatibility` | `DYNAMIC_SECTION` over SIMPL 6.4 + 6.5; validates UUID + arg-key + param-value decoding | + +All 9 TEST_CASEs pass. + +## Exemplar archive + +None — all fixtures constructed in C++ at test time. Provenance in `vv/provenance/ComputeFeatureSizesFilter.md`. + +## Deviations from DREAM3D 6.5.171 + +**ImageGeom path:** No deviations. Formulas and logic identical to `FindSizes::execute()`. The float64 intermediate in the ESD/ECD computation differs from likely float32 in legacy by ≤1 ULP for typical EBSD spacings; non-material. + +**RectGridGeom path — `ComputeFeatureSizes-D1`:** Per-feature volumes differ from `FindSizes::findSizesUnstructured()` output due to two precision improvements in SIMPLNX: (1) element sizes promoted from float32 to float64 before accumulation; (2) Kahan compensated summation applied per-thread and during TBB post-reduction, reducing accumulated error from O(N·ε_float32) to O(ε_float64). SIMPLNX is more accurate. Users migrating from DREAM3D 6.5.171 should expect small shifts in per-feature volumes for RectGridGeom; largest for features with many cells on grids with high cell-volume variation. + +Full entry in `vv/deviations/ComputeFeatureSizesFilter.md`. diff --git a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md new file mode 100644 index 0000000000..52652adaec --- /dev/null +++ b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md @@ -0,0 +1,47 @@ +# Deviations from DREAM3D 6.5.171: ComputeFeatureSizes + +This file lists every documented behavioral difference between this SIMPLNX filter and its DREAM3D 6.5.171 equivalent. + +Entries are referenced by stable ID (`ComputeFeatureSizes-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. + +--- + +## ImageGeom path — no deviations + +Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `FindSizes::execute()` confirmed identical algorithmic structure: + +- Per-feature voxel count: identical parallel accumulation logic (serial in legacy). +- 3D volume formula: `volume = voxelCount × voxelVolume`; ESD: `2·∛(volume / (4π/3))` — both use the standard sphere formula. +- 2D area formula (any dim == 1): `area = voxelCount × voxelArea`; ECD: `2·√(area / π)` — both use the standard circle formula. + +**Precision non-deviation (not flagged):** SIMPLNX promotes `voxelVolume` to `float64` and evaluates `cbrt` / `sqrt` in `float64` before casting to `float32` for storage. The legacy filter likely used `float32` throughout. The difference is within ~1 ULP of `float32` for typical EBSD spacings and is non-material for downstream morphological statistics. + +--- + +## ComputeFeatureSizes-D1 + +| Field | Value | +|---|---| +| **Deviation ID** | `ComputeFeatureSizes-D1` | +| **Filter UUID** | `c666ee17-ca58-4969-80d0-819986c72485` | +| **Status** | Active — precision improvement; SIMPLNX output is more accurate | + +**Symptom:** Per-feature volumes for features in `RectGridGeom` geometries differ from DREAM3D 6.5.171 `FindSizes::findSizesUnstructured()` output. The discrepancy grows with feature size and with the variation in cell volumes across the grid, scaling approximately as `O(N · ε_float32)` for the legacy result vs `O(ε_float64)` for the SIMPLNX result, where N is the number of cells in the feature. + +**Root cause:** Precision — two compounding improvements in SIMPLNX's `ProcessRectGridGeom` over the legacy `findSizesUnstructured`: + +1. **float32 → float64 promotion.** Element sizes are stored as `float32` (the output of `findElementSizes`). The legacy `findSizesUnstructured` accumulated these directly in `float32`, so each per-cell addition carried a relative error of ~`ε_float32 ≈ 1.2×10⁻⁷`. SIMPLNX promotes each element size to `float64` before accumulation (`static_cast(m_ElemSizes.getValue(voxelIdx))`), reducing the per-operation rounding error to `ε_float64 ≈ 2.2×10⁻¹⁶`. + +2. **Kahan summation.** SIMPLNX applies Kahan compensated summation at two levels: + - *Per-thread, per-cell* in `RectGridSummationImpl::convert()` (lines 274–283): standard Kahan with `y = elemSize - c`, `t = sum + y`, `c′ = (t - sum) - y`. For a feature spanning N cells, naive float64 accumulation still has O(N · ε_float64) error; Kahan reduces this to O(ε_float64) by carrying a compensation term `c` that recovers the low-order bits lost in each `sum + y` operation. + - *Post-reduction, per thread-local vector* in `threadLocalVolumes.combine_each()` (lines 341–358): the same Kahan scheme is applied when combining the TBB thread-local partial sums into the final per-feature volume. + + For a non-uniform rectilinear grid where cell volumes span several orders of magnitude (e.g., a grid with fine near-surface resolution and coarse interior), naive summation of N cells can lose ~`log₂(V_max / V_min)` bits of precision in the smaller cell contributions. Kahan summation recovers those bits regardless of N or the dynamic range of cell volumes. + +**Affected users:** Any workflow that runs `ComputeFeatureSizes` on a `RectGridGeom` and then compares per-feature volume or ESD values against DREAM3D 6.5.171 output. The deviation is largest for large features (high N) on grids with high volume variation. On uniform RectGrids (all cell volumes equal), Kahan has no practical effect and the only deviation is the float32→float64 promotion. + +**Recommendation:** Trust SIMPLNX. The legacy `findSizesUnstructured` accumulated unnecessary floating-point error that grew with feature size and grid non-uniformity. The SIMPLNX result is strictly more accurate. Users migrating pipelines should expect small positive or negative shifts in per-feature volumes; ESD values are affected proportionally through the `cbrt` formula. + +--- + +*If a future comparison run against DREAM3D 6.5.171 output reveals additional deviations, add them here as `ComputeFeatureSizes-D2` etc.* From f3a3711c62f88f6886d5ecb7f0634b10b703f07b Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Sat, 27 Jun 2026 14:49:44 -0400 Subject: [PATCH 4/9] =?UTF-8?q?DOC:=20ComputeFeatureSizes=20V&V=20?= =?UTF-8?q?=E2=80=94=20add=20D2=20(ESD=20evaluation)=20and=20A/B=20verific?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A direct A/B run (stock 6.5.171 vs DREAM3D-NX vs a 6.5.172 proof-patch build) on the exact RectGrid fixture showed the legacy-vs-SIMPLNX difference has two independent precision sources, not one: * D1 (Volumes): float64 + Kahan summation vs legacy naive float32 accumulation. Patching only this made Volumes bit-identical to SIMPLNX. * D2 (EquivalentDiameters): float64 std::cbrt on the float64 volume vs legacy powf on the float32-rounded volume. A second, independent source — even with identical Volumes, ESD still differed by ~1 ULP until this was also patched. Both changes together make legacy reproduce SIMPLNX output bit-for-bit. The report and deviations file are updated to document D2, mark D1/D2 as A/B-verified, and correct the prior text that treated the ESD difference as proportional to the volume shift rather than a separate deviation. Signed-off-by: Michael Jackson --- .../vv/ComputeFeatureSizesFilter.md | 10 +++--- .../deviations/ComputeFeatureSizesFilter.md | 34 +++++++++++++++++-- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md index 83e4670bf5..b246e7004c 100644 --- a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md @@ -18,12 +18,12 @@ | Code paths | **14 of 19** exercised; 5 gaps (cancel ×2, INT32_MAX overflow ×2, other-geom no-op ×1) | | Tests | **9 TEST_CASEs** — all pass | | External archive | None | -| Deviations | **1 active** (`ComputeFeatureSizes-D1`): Kahan vs naive summation in RectGridGeom — see deviations file | +| Deviations | **2 active**, both A/B-verified 2026-06-27: `ComputeFeatureSizes-D1` (float64+Kahan vs naive summation → `Volumes`), `ComputeFeatureSizes-D2` (float64 `std::cbrt` vs float32 `powf` → `EquivalentDiameters`) — see deviations file | | Open bugs | **1 open** (`Bug-1`): 2D area formula uses all 3 spacings (= volume) instead of the 2 non-flat spacings | ## Summary -`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. One precision deviation (`D1`) and one open bug (`Bug-1`) are documented below. +`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. Two precision deviations (`D1`, `D2`) — both confirmed by a direct A/B run against DREAM3D 6.5.171 — and one open bug (`Bug-1`) are documented below. ## Algorithm Relationship @@ -99,8 +99,10 @@ None — all fixtures constructed in C++ at test time. Provenance in `vv/provena ## Deviations from DREAM3D 6.5.171 -**ImageGeom path:** No deviations. Formulas and logic identical to `FindSizes::execute()`. The float64 intermediate in the ESD/ECD computation differs from likely float32 in legacy by ≤1 ULP for typical EBSD spacings; non-material. +Both deviations below were confirmed by a direct A/B run (2026-06-27), not source inspection alone: the exact RectGrid fixture was authored as a shared legacy `.dream3d` and run through stock DREAM3D 6.5.171, DREAM3D-NX, and a 6.5.172 proof-patch build. Applying **both** the D1 (summation) and D2 (ESD-evaluation) changes to legacy `findSizesUnstructured` made `Volumes` and `EquivalentDiameters` **bit-identical** to SIMPLNX; each change alone closed only its corresponding array. -**RectGridGeom path — `ComputeFeatureSizes-D1`:** Per-feature volumes differ from `FindSizes::findSizesUnstructured()` output due to two precision improvements in SIMPLNX: (1) element sizes promoted from float32 to float64 before accumulation; (2) Kahan compensated summation applied per-thread and during TBB post-reduction, reducing accumulated error from O(N·ε_float32) to O(ε_float64). SIMPLNX is more accurate. Users migrating from DREAM3D 6.5.171 should expect small shifts in per-feature volumes for RectGridGeom; largest for features with many cells on grids with high cell-volume variation. +**`ComputeFeatureSizes-D1` (RectGridGeom → `Volumes`):** Per-feature volumes differ from `FindSizes::findSizesUnstructured()` output due to two precision improvements in SIMPLNX: (1) element sizes promoted from float32 to float64 before accumulation; (2) Kahan compensated summation applied per-thread and during TBB post-reduction, reducing accumulated error from O(N·ε_float32) to O(ε_float64). SIMPLNX is more accurate. Users migrating from DREAM3D 6.5.171 should expect small shifts in per-feature volumes for RectGridGeom; largest for features with many cells on grids with high cell-volume variation. + +**`ComputeFeatureSizes-D2` (RectGridGeom → `EquivalentDiameters`):** Even with identical `Volumes`, the equivalent spherical diameter differs because legacy evaluates the cube root with `powf` (float32) on the float32-rounded volume, while SIMPLNX uses `std::cbrt` (float64) on the float64 volume. SIMPLNX is more accurate. The same `powf`/`sqrtf`-on-float32 pattern exists on the ImageGeom path; it was previously noted only as a non-flagged precision difference and is now captured by D2. Full entry in `vv/deviations/ComputeFeatureSizesFilter.md`. diff --git a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md index 52652adaec..71a7f7c485 100644 --- a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md @@ -24,7 +24,8 @@ Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `Find |---|---| | **Deviation ID** | `ComputeFeatureSizes-D1` | | **Filter UUID** | `c666ee17-ca58-4969-80d0-819986c72485` | -| **Status** | Active — precision improvement; SIMPLNX output is more accurate | +| **Affected array** | `Volumes` (RectGridGeom) | +| **Status** | Active — precision improvement; SIMPLNX output is more accurate. **Verified by A/B (2026-06-27)** — see "A/B verification" below. | **Symptom:** Per-feature volumes for features in `RectGridGeom` geometries differ from DREAM3D 6.5.171 `FindSizes::findSizesUnstructured()` output. The discrepancy grows with feature size and with the variation in cell volumes across the grid, scaling approximately as `O(N · ε_float32)` for the legacy result vs `O(ε_float64)` for the SIMPLNX result, where N is the number of cells in the feature. @@ -40,8 +41,35 @@ Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `Find **Affected users:** Any workflow that runs `ComputeFeatureSizes` on a `RectGridGeom` and then compares per-feature volume or ESD values against DREAM3D 6.5.171 output. The deviation is largest for large features (high N) on grids with high volume variation. On uniform RectGrids (all cell volumes equal), Kahan has no practical effect and the only deviation is the float32→float64 promotion. -**Recommendation:** Trust SIMPLNX. The legacy `findSizesUnstructured` accumulated unnecessary floating-point error that grew with feature size and grid non-uniformity. The SIMPLNX result is strictly more accurate. Users migrating pipelines should expect small positive or negative shifts in per-feature volumes; ESD values are affected proportionally through the `cbrt` formula. +**Recommendation:** Trust SIMPLNX. The legacy `findSizesUnstructured` accumulated unnecessary floating-point error that grew with feature size and grid non-uniformity. The SIMPLNX result is strictly more accurate. Users migrating pipelines should expect small positive or negative shifts in per-feature volumes. The `EquivalentDiameters` array is affected both by this volume shift **and** by a separate ESD-evaluation deviation — see `ComputeFeatureSizes-D2`. + +**A/B verification (2026-06-27):** A direct comparison was run, not just source inspection. The exact RectGrid unit-test fixture was authored as a shared legacy `.dream3d` and run through stock DREAM3D 6.5.171, DREAM3D-NX, and a 6.5.172 proof-patch build. Stock 6.5.171 `Volumes` differed from SIMPLNX (≈1 float32 ULP on this small fixture; grows with N). Applying **only** the float64 + Kahan summation change to legacy `findSizesUnstructured` made `Volumes` **bit-identical** to SIMPLNX, confirming summation precision as the sole root cause of the `Volumes` deviation. + +--- + +## ComputeFeatureSizes-D2 + +| Field | Value | +|---|---| +| **Deviation ID** | `ComputeFeatureSizes-D2` | +| **Filter UUID** | `c666ee17-ca58-4969-80d0-819986c72485` | +| **Affected array** | `EquivalentDiameters` (RectGridGeom; same evaluation pattern on ImageGeom) | +| **Status** | Active — precision improvement; SIMPLNX output is more accurate. **Verified by A/B (2026-06-27).** | + +**Symptom:** Per-feature `EquivalentDiameters` for `RectGridGeom` geometries differ from DREAM3D 6.5.171 output by ≈1 float32 ULP **even after the `Volumes` arrays are made identical**. This is a second precision source, independent of the summation in D1. + +**Root cause:** The equivalent spherical diameter evaluation differs: +- **Legacy `findSizesUnstructured`:** `rad = m_Volumes[i] / vol_term; diameter = 2.0f * powf(rad, 0.3333333333f)` — the cube root is taken with `powf` (float32) on the **float32-rounded** stored volume, and `vol_term` is `(4/3)·k_Pif` (float32 π). +- **SIMPLNX `ProcessRectGridGeom`:** `2.0 * std::cbrt(featureVolumes[i] / k_ESDVolumeDenominator)` — `std::cbrt` (float64) on the **float64** accumulated volume, with `k_ESDVolumeDenominator = (4·π)/3` in float64. + +So even with identical `Volumes`, the ESD differs because the legacy path rounds the volume to float32 first and uses a float32 `powf` cube root, while SIMPLNX evaluates the cube root in float64 on the unrounded sum. + +**A/B verification (2026-06-27):** After making `Volumes` identical (D1 patch), `EquivalentDiameters` still differed by ≈1 ULP. Additionally changing legacy to compute the diameter as `2.0 * std::cbrt(volumeSums[i] / ((4.0·k_Pi)/3.0))` (double `std::cbrt` on the double sum) made `EquivalentDiameters` **bit-identical** to SIMPLNX. Both changes together — D1 (summation) and D2 (ESD evaluation) — are required for legacy to fully reproduce SIMPLNX output. + +**Affected users:** Any workflow comparing `EquivalentDiameters` against DREAM3D 6.5.171. The same `powf`-on-float32 vs `cbrt`-on-float64 pattern exists on the ImageGeom path (`FindSizes::findSizesImage` uses `powf`/`sqrtf`); it was previously noted only as a non-flagged precision difference and is folded into this entry for the equivalent-diameter/ECD evaluation. + +**Recommendation:** Trust SIMPLNX. The float64 `std::cbrt`/`std::sqrt` evaluation is more accurate than the legacy float32 `powf`/`sqrtf` on a pre-rounded value. --- -*If a future comparison run against DREAM3D 6.5.171 output reveals additional deviations, add them here as `ComputeFeatureSizes-D2` etc.* +*If a future comparison run against DREAM3D 6.5.171 output reveals additional deviations, add them here as `ComputeFeatureSizes-D3` etc.* From 6e70ccde01034fe6426a7c9df0e3a7f25d3da92c Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Wed, 8 Jul 2026 15:48:47 -0400 Subject: [PATCH 5/9] BUG: Fix ComputeFeatureSizes 2D area formula; review fixes Fixes from adversarial review of this PR: * Fix Bug-1: the 2D (one-empty-dimension) voxel area multiplied all three spacings, computing a volume instead of an area, and diverged from DREAM3D 6.5.171 whenever the flat dimension's spacing was not 1.0. The area now uses only the two non-flat spacings, matching legacy FindSizes::findSizesImage. The prior 2D fixture used a flat-dim spacing of exactly 1.0, hiding the defect. With this fix the deviations file's "ImageGeom path - no deviations" claim is once again correct. * Add a characterization test (2D area excludes the flat-dimension spacing) that GENERATEs all three flat orientations with a non-unit flat-dim spacing of 5.0 and asserts area 24.0, not 120.0. * Gate the ImageGeom and RectGrid parallel scans with requireStoresInMemory on featureIds: the workers read the store concurrently across slices, which the project thread-safety policy only permits for in-core stores. * Correct the stale provenance canonical oracle block (Volumes[1] 2358.834 -> 2362.434, EquivalentDiameters 16.516/3.0994 -> 16.5242/ 3.0669) to agree with the tests and the provenance's own per-slice trace; fix the matching stale comment in the RectGrid fixture. * Clear the report's "1 open bug" flag (Bug-1 now fixed) and document the retired "Legacy: Small IN100 Test" in the test inventory. * Qualify the D1 Kahan claim: the compensator resets per TBB chunk, so the error reduction is per-chunk, not a full continuous-Kahan O(eps_f64); the float64 accumulation is the dominant improvement. Signed-off-by: Michael Jackson --- .../Algorithms/ComputeFeatureSizes.cpp | 31 ++++++++++- .../test/ComputeFeatureSizesTest.cpp | 55 ++++++++++++++++++- .../vv/ComputeFeatureSizesFilter.md | 36 ++++++------ .../deviations/ComputeFeatureSizesFilter.md | 4 +- .../provenance/ComputeFeatureSizesFilter.md | 4 +- 5 files changed, 106 insertions(+), 24 deletions(-) diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp index 565e30ee66..0fa5a778d6 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp @@ -93,6 +93,12 @@ Result<> ProcessImageGeom(ImageGeom& imageGeom, Float32AbstractDataStore& volume FeatureVoxelCountsT threadLocalVoxelCounts([numFeatures] { return std::vector(numFeatures, 0); }); ParallelDataAlgorithm dataAlg; dataAlg.setRange(0, dims[2]); + // The worker reads featureIds concurrently across slices; per the project thread-safety policy a + // DataStore is only safe for concurrent access when it is resident in memory, so gate parallelization + // on that (an out-of-core store falls back to serial execution). + IParallelAlgorithm::AlgorithmStores algStores; + algStores.push_back(&featureIds); + dataAlg.requireStoresInMemory(algStores); dataAlg.execute(ImageSummationImpl(threadLocalVoxelCounts, dims, featureIds, shouldCancel)); if(shouldCancel) @@ -142,8 +148,24 @@ Result<> ProcessImageGeom(ImageGeom& imageGeom, Float32AbstractDataStore& volume * For these two cases the following code would BREAK, so do not enable. **/ - // Calculate the area of a single voxel - const float64 voxelArea = static_cast(spacing[0]) * static_cast(spacing[1]) * static_cast(spacing[2]); + // Calculate the area of a single voxel from the two NON-flat dimensions only. Exactly one dimension + // is flat here (preflight rejects more than one empty dimension), so its spacing must be excluded -- + // including it would compute a volume, not an area, and would diverge from DREAM3D 6.5.171 whenever + // the flat dimension's spacing != 1. This matches legacy FindSizes::findSizesImage, which uses only + // the two non-flat resolutions. + float64 voxelArea = 1.0; + if(xDimSize != 1) + { + voxelArea *= static_cast(spacing[0]); + } + if(yDimSize != 1) + { + voxelArea *= static_cast(spacing[1]); + } + if(zDimSize != 1) + { + voxelArea *= static_cast(spacing[2]); + } msgHelper.sendMessage("Feature Level: Storing Voxel Counts and Calculating Area and ECD..."); // Process each feature storing feature voxel counts, areas, and equivalent circular diameter @@ -326,6 +348,11 @@ Result<> ProcessRectGridGeom(RectGridGeom& rectGridGeom, Float32AbstractDataStor FeatureVolumesT threadLocalVolumes([numFeatures] { return std::vector(numFeatures, 0); }); ParallelDataAlgorithm dataAlg; dataAlg.setRange(0, dims[2]); + // The worker reads featureIds concurrently across slices; gate parallelization on the store being + // resident in memory (see the ProcessImageGeom note; project thread-safety policy). + IParallelAlgorithm::AlgorithmStores algStores; + algStores.push_back(&featureIds); + dataAlg.requireStoresInMemory(algStores); dataAlg.execute(RectGridSummationImpl(threadLocalVoxelCounts, threadLocalVolumes, dims, featureIds, elemSizes, shouldCancel)); if(shouldCancel) diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index 27933bbfe3..5eeb80f240 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -225,8 +225,8 @@ DataStructure CreateRectGridDataStructure() // clang-format off // Expected Outputs: // numElements: 0 39 15 10 - // volumes: 0.0 2358.834 352.462 15.104 - // eqDiameters: 0.0 16.516 8.764 3.0994 + // volumes: 0.0 2362.434 352.462 15.104 + // eqDiameters: 0.0 16.5242 8.76404 3.06689 const std::array featureIdsArray = { 1, 2, 2, 2, 1, 1, 1, 1, @@ -319,6 +319,57 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: Valid: Image 2D", "[SimplnxCore][Co UnitTest::CheckArraysInheritTupleDims(dataStructure); } +// Characterization test for the 2D area formula: the flat dimension's spacing must be EXCLUDED so a +// 2D area is a true area, not a volume. The existing "Valid: Image 2D" fixture uses a flat-dimension +// spacing of 1.0, which hides an all-three-spacings bug; here the flat dimension's spacing is +// deliberately non-unit (5.0). If the flat spacing were (incorrectly) included, every area would be +// 5x too large. All three flat orientations are checked so the fix cannot be an axis-specific special +// case. This matches DREAM3D 6.5.171, which uses only the two non-flat resolutions. +TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension spacing", "[SimplnxCore][ComputeFeatureSizes]") +{ + auto [label, dims, spacing] = GENERATE(std::make_tuple("flat Z", SizeVec3{2, 2, 1}, FloatVec3{2.0f, 3.0f, 5.0f}), std::make_tuple("flat X", SizeVec3{1, 2, 2}, FloatVec3{5.0f, 2.0f, 3.0f}), + std::make_tuple("flat Y", SizeVec3{2, 1, 2}, FloatVec3{2.0f, 5.0f, 3.0f})); + + DYNAMIC_SECTION(label) + { + // Four cells, all feature 1 (feature 0 unused). Non-flat spacings are always 2.0 and 3.0, so the + // per-voxel area is 6.0 and the single feature's area is 4 * 6.0 = 24.0 regardless of which axis is + // flat. The flat dimension's spacing is 5.0 and must NOT appear in the product. + DataStructure dataStructure; + auto* imageGeom = ImageGeom::Create(dataStructure, Test::k_ImageGeomName); + imageGeom->setSpacing(spacing); + imageGeom->setOrigin(FloatVec3{0.0f, 0.0f, 0.0f}); + imageGeom->setDimensions(dims); + + const ShapeType tupleShape{dims[2], dims[1], dims[0]}; + auto* cellData = AttributeMatrix::Create(dataStructure, Test::k_CellAMName, tupleShape, imageGeom->getId()); + imageGeom->setCellData(*cellData); + auto* featureIds = Int32Array::CreateWithStore(dataStructure, Test::k_FeatureIdsName, cellData->getShape(), ShapeType{1}, cellData->getId()); + featureIds->fill(1); + AttributeMatrix::Create(dataStructure, Test::k_FeatureAMName, ShapeType{2}, imageGeom->getId()); + + ComputeFeatureSizesFilter filter; + Arguments args; + args.insert(ComputeFeatureSizesFilter::k_GeometryPath_Key, std::make_any(Test::k_ImageGeomPath)); + args.insert(ComputeFeatureSizesFilter::k_SaveElementSizes_Key, std::make_any(false)); + args.insert(ComputeFeatureSizesFilter::k_CellFeatureIdsArrayPath_Key, std::make_any(Test::k_FeatureIdsPath)); + args.insert(ComputeFeatureSizesFilter::k_CellFeatureAttributeMatrixPath_Key, std::make_any(Test::k_FeatureAMPath)); + args.insert(ComputeFeatureSizesFilter::k_VolumesName_Key, std::make_any(Test::k_VolumesName)); + args.insert(ComputeFeatureSizesFilter::k_EquivalentDiametersName_Key, std::make_any(Test::k_EquivalentDiametersName)); + args.insert(ComputeFeatureSizesFilter::k_NumElementsName_Key, std::make_any(Test::k_NumElementsName)); + + auto preflightResult = filter.preflight(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions); + auto executeResult = filter.execute(dataStructure, args); + SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result); + + const auto& areas = dataStructure.getDataRefAs(Test::k_VolumesPath); + REQUIRE(areas.getValue(1) == Approx(24.0f)); // 4 voxels * (2.0 * 3.0); NOT 4 * (2.0 * 3.0 * 5.0) = 120 + + UnitTest::CheckArraysInheritTupleDims(dataStructure); + } +} + TEST_CASE("SimplnxCore::ComputeFeatureSizes: Valid: Image 2D with Element Sizes", "[SimplnxCore][ComputeFeatureSizes]") { DataStructure dataStructure = Test::Create2DImageDataStructure(); diff --git a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md index b246e7004c..08226b711c 100644 --- a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md @@ -16,14 +16,14 @@ | Algorithm relationship | **Port** of both `FindSizes::execute()` (ImageGeom) and `FindSizes::findSizesUnstructured()` (RectGridGeom) | | Oracle | **Class 1 (Analytical)** — all test data inlined, hand-derived | | Code paths | **14 of 19** exercised; 5 gaps (cancel ×2, INT32_MAX overflow ×2, other-geom no-op ×1) | -| Tests | **9 TEST_CASEs** — all pass | +| Tests | **10 TEST_CASEs** — all pass | | External archive | None | | Deviations | **2 active**, both A/B-verified 2026-06-27: `ComputeFeatureSizes-D1` (float64+Kahan vs naive summation → `Volumes`), `ComputeFeatureSizes-D2` (float64 `std::cbrt` vs float32 `powf` → `EquivalentDiameters`) — see deviations file | -| Open bugs | **1 open** (`Bug-1`): 2D area formula uses all 3 spacings (= volume) instead of the 2 non-flat spacings | +| Open bugs | **None.** `Bug-1` (2D area formula multiplied all 3 spacings instead of the 2 non-flat spacings) was **fixed this cycle** and pinned by the `2D area excludes the flat-dimension spacing` characterization test; the ImageGeom 2D path now matches legacy (no deviation). | ## Summary -`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. Two precision deviations (`D1`, `D2`) — both confirmed by a direct A/B run against DREAM3D 6.5.171 — and one open bug (`Bug-1`) are documented below. +`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. Two precision deviations (`D1`, `D2`) — both confirmed by a direct A/B run against DREAM3D 6.5.171 — are documented below. A latent 2D-area bug (`Bug-1`: the flat dimension's spacing was included in the per-voxel area) was found during this V&V and **fixed**; the 2D path now matches legacy and is pinned by a dedicated characterization test with a non-unit flat-dimension spacing. ## Algorithm Relationship @@ -79,19 +79,21 @@ Second-engineer review pending: verify hand-derivations for all three fixtures a ## Test inventory -| Test case | Notes | -|---|---| -| `Valid: Image 2D` | Class 1; 5×5×1, spacing 20.2×0.1×1.0; SaveElementSizes=false | -| `Valid: Image 2D with Element Sizes` | Same fixture; SaveElementSizes=true | -| `Valid: Image Stack 3D` | Class 1; 5×5×5, spacing 1.2×0.9×2.1; SaveElementSizes=false | -| `Valid: Image Stack 3D with Element Size` | Same fixture; SaveElementSizes=true | -| `Valid: Rectilinear Grid` | Class 1; 4×4×4 non-uniform; SaveElementSizes=false; Kahan path exercised | -| `Valid: Rectilinear Grid with Element Size` | Same fixture; SaveElementSizes=true | -| `Invalid: Execution Failure` | FeatureId 10 in a 4-feature AM; asserts execute result invalid | -| `Invalid: Preflight Failure` | 4 degenerate dim configs; asserts preflight result invalid | -| `SIMPL Backwards Compatibility` | `DYNAMIC_SECTION` over SIMPL 6.4 + 6.5; validates UUID + arg-key + param-value decoding | - -All 9 TEST_CASEs pass. +| Test case | Status | Notes | +|---|---|---| +| `Valid: Image 2D` | kept | Class 1; 5×5×1, spacing 20.2×0.1×1.0 (flat-Z spacing 1.0); SaveElementSizes=false | +| `2D area excludes the flat-dimension spacing` | new-for-V&V | Class 1 characterization pin for the Bug-1 fix. 2×2 slab, non-flat spacings 2.0×3.0, flat-dimension spacing **5.0**; asserts area == 24.0 (not 120.0). `GENERATE`s all three flat orientations (X/Y/Z) so the flat spacing is proven excluded regardless of axis. | +| `Valid: Image 2D with Element Sizes` | kept | Same fixture as `Valid: Image 2D`; SaveElementSizes=true | +| `Valid: Image Stack 3D` | kept | Class 1; 5×5×5, spacing 1.2×0.9×2.1; SaveElementSizes=false | +| `Valid: Image Stack 3D with Element Size` | kept | Same fixture; SaveElementSizes=true | +| `Valid: Rectilinear Grid` | kept | Class 1; 4×4×4 non-uniform; SaveElementSizes=false; Kahan path exercised | +| `Valid: Rectilinear Grid with Element Size` | kept | Same fixture; SaveElementSizes=true | +| `Invalid: Execution Failure` | kept | FeatureId 10 in a 4-feature AM; asserts execute result invalid | +| `Invalid: Preflight Failure` | kept | 4 degenerate dim configs; asserts preflight result invalid | +| `SIMPL Backwards Compatibility` | kept | `DYNAMIC_SECTION` over SIMPL 6.4 + 6.5; validates UUID + arg-key + param-value decoding | +| `Legacy: Small IN100 Test` | retired | Real-data comparison against legacy-produced `6_6_stats_test_v2.dream3d` arrays. Retired because it was a legacy-output regression check (not an independent oracle) and the D1/D2 precision deviations intentionally change those exact values; the RectGrid A/B (deviations file) now provides the legacy comparison and the inline Class 1 fixtures provide the correctness oracle. | + +All 10 TEST_CASEs pass. ## Exemplar archive @@ -101,7 +103,7 @@ None — all fixtures constructed in C++ at test time. Provenance in `vv/provena Both deviations below were confirmed by a direct A/B run (2026-06-27), not source inspection alone: the exact RectGrid fixture was authored as a shared legacy `.dream3d` and run through stock DREAM3D 6.5.171, DREAM3D-NX, and a 6.5.172 proof-patch build. Applying **both** the D1 (summation) and D2 (ESD-evaluation) changes to legacy `findSizesUnstructured` made `Volumes` and `EquivalentDiameters` **bit-identical** to SIMPLNX; each change alone closed only its corresponding array. -**`ComputeFeatureSizes-D1` (RectGridGeom → `Volumes`):** Per-feature volumes differ from `FindSizes::findSizesUnstructured()` output due to two precision improvements in SIMPLNX: (1) element sizes promoted from float32 to float64 before accumulation; (2) Kahan compensated summation applied per-thread and during TBB post-reduction, reducing accumulated error from O(N·ε_float32) to O(ε_float64). SIMPLNX is more accurate. Users migrating from DREAM3D 6.5.171 should expect small shifts in per-feature volumes for RectGridGeom; largest for features with many cells on grids with high cell-volume variation. +**`ComputeFeatureSizes-D1` (RectGridGeom → `Volumes`):** Per-feature volumes differ from `FindSizes::findSizesUnstructured()` output due to two precision improvements in SIMPLNX: (1) element sizes promoted from float32 to float64 before accumulation; (2) Kahan compensated summation. Note the Kahan compensator is a local reset on each TBB body invocation (the `combinable` per-thread volume persists, but the compensation term does not carry across chunk boundaries within a thread), so the accumulated-error reduction is per-chunk rather than the full O(ε_float64) a single continuous Kahan pass would give; the dominant improvement is the float64 accumulation. SIMPLNX is still more accurate than the legacy naive float32 sum. Users migrating from DREAM3D 6.5.171 should expect small shifts in per-feature volumes for RectGridGeom; largest for features with many cells on grids with high cell-volume variation. **`ComputeFeatureSizes-D2` (RectGridGeom → `EquivalentDiameters`):** Even with identical `Volumes`, the equivalent spherical diameter differs because legacy evaluates the cube root with `powf` (float32) on the float32-rounded volume, while SIMPLNX uses `std::cbrt` (float64) on the float64 volume. SIMPLNX is more accurate. The same `powf`/`sqrtf`-on-float32 pattern exists on the ImageGeom path; it was previously noted only as a non-flagged precision difference and is now captured by D2. diff --git a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md index 71a7f7c485..0e2bea962c 100644 --- a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md @@ -12,7 +12,9 @@ Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `Find - Per-feature voxel count: identical parallel accumulation logic (serial in legacy). - 3D volume formula: `volume = voxelCount × voxelVolume`; ESD: `2·∛(volume / (4π/3))` — both use the standard sphere formula. -- 2D area formula (any dim == 1): `area = voxelCount × voxelArea`; ECD: `2·√(area / π)` — both use the standard circle formula. +- 2D area formula (exactly one dim == 1): `area = voxelCount × (product of the two NON-flat spacings)`; ECD: `2·√(area / π)` — both use the standard circle formula, and both exclude the flat dimension's spacing. + +> **Corrected this V&V cycle (was a latent SIMPLNX bug, now no deviation):** `ProcessImageGeom` previously multiplied **all three** spacings for the 2D voxel area (`spacing[0]·spacing[1]·spacing[2]`), which is a volume, not an area. It diverged from DREAM3D 6.5.171 (which uses only the two non-flat resolutions) whenever the flat dimension's spacing was not 1.0. The prior 2D unit fixture used a flat-dimension spacing of exactly 1.0, so the defect was invisible. The formula now excludes the flat dimension's spacing, restoring parity with legacy — hence this remains a *no-deviation* entry. Pinned by the `2D area excludes the flat-dimension spacing` characterization test (flat X/Y/Z, non-unit flat spacing). **Precision non-deviation (not flagged):** SIMPLNX promotes `voxelVolume` to `float64` and evaluates `cbrt` / `sqrt` in `float64` before casting to `float32` for storage. The legacy filter likely used `float32` throughout. The difference is within ~1 ULP of `float32` for typical EBSD spacings and is non-material for downstream morphological statistics. diff --git a/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md index d1466a0898..e6daec9ece 100644 --- a/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/provenance/ComputeFeatureSizesFilter.md @@ -64,8 +64,8 @@ The expected outputs are as follows: ```text NumElements: {0, 39, 15, 10} -Volumes: {0.0, 2358.834, 352.462, 15.104} -EquivalentDiameters: {0.0, 16.516, 8.764, 3.0994} +Volumes: {0.0, 2362.434, 352.462, 15.104} +EquivalentDiameters: {0.0, 16.5242, 8.764, 3.0669} ``` In order to derive these values, the voxels must be processed individually and summed up. This is how it was done: From 0df5f2c8c944e3cffaf67a8a0ab8b3783a359c7c Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Thu, 16 Jul 2026 16:11:28 -0400 Subject: [PATCH 6/9] TEST: Use relative tolerance for ComputeFeatureSizes oracle assertions * Replace absolute float32-epsilon comparisons (which demanded bit-exact equality at magnitudes where one ULP far exceeds epsilon) with Approx().epsilon(1e-6) relative comparisons * A few ULPs of slack keeps the oracle pins meaningful while surviving platform and TBB reduction-order differences Signed-off-by: Michael Jackson --- .../test/ComputeFeatureSizesTest.cpp | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index 5eeb80f240..c7a7fe4e16 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -15,6 +15,11 @@ namespace fs = std::filesystem; namespace Test { +// Relative tolerance for comparing float32 results against the hand-derived oracle values. A few +// float32 ULPs (~1.2e-7 each) of slack so the pins survive platform and TBB reduction-order +// differences without demanding bit-exact equality; anything beyond this is a real deviation. +constexpr float64 k_RelativeTolerance = 1.0e-6; + // Geometry Level const std::string k_ImageGeomName = "Image"; const DataPath k_ImageGeomPath = DataPath({k_ImageGeomName}); @@ -87,14 +92,14 @@ void Validate2DImageDataStructure(const DataStructure& dataStructure) REQUIRE(numElements.getValue(3) == 13); // areas: 0.0 22.22 2.02 26.26 const auto& areas = dataStructure.getDataRefAs(k_VolumesPath); - REQUIRE(std::abs(areas.getValue(1) - 22.220001f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(areas.getValue(2) - 2.0200002f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(areas.getValue(3) - 26.260002f) < std::numeric_limits::epsilon()); + REQUIRE(areas.getValue(1) == Approx(22.220001f).epsilon(k_RelativeTolerance)); + REQUIRE(areas.getValue(2) == Approx(2.0200002f).epsilon(k_RelativeTolerance)); + REQUIRE(areas.getValue(3) == Approx(26.260002f).epsilon(k_RelativeTolerance)); // eqDiameters: 0.0 5.318964 1.603728 5.78232 const auto& equivalentDiameters = dataStructure.getDataRefAs(k_EquivalentDiametersPath); - REQUIRE(std::abs(equivalentDiameters.getValue(1) - 5.3189644f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(2) - 1.60372818f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(3) - 5.7823243f) < std::numeric_limits::epsilon()); + REQUIRE(equivalentDiameters.getValue(1) == Approx(5.3189644f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(2) == Approx(1.60372818f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(3) == Approx(5.7823243f).epsilon(k_RelativeTolerance)); } DataStructure Create3DImageDataStructure() @@ -171,14 +176,14 @@ void Validate3DImageDataStructure(const DataStructure& dataStructure) REQUIRE(numElements.getValue(3) == 23); // volumes: 0.0 165.564 65.772 52.164 const auto& volumes = dataStructure.getDataRefAs(k_VolumesPath); - REQUIRE(std::abs(volumes.getValue(1) - 165.564f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(volumes.getValue(2) - 65.771995f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(volumes.getValue(3) - 52.163997f) < std::numeric_limits::epsilon()); + REQUIRE(volumes.getValue(1) == Approx(165.564f).epsilon(k_RelativeTolerance)); + REQUIRE(volumes.getValue(2) == Approx(65.771995f).epsilon(k_RelativeTolerance)); + REQUIRE(volumes.getValue(3) == Approx(52.163997f).epsilon(k_RelativeTolerance)); // eqDiameters: 0.0 6.81275 5.00819 4.63579 const auto& equivalentDiameters = dataStructure.getDataRefAs(k_EquivalentDiametersPath); - REQUIRE(std::abs(equivalentDiameters.getValue(1) - 6.8127493f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(2) - 5.0081901f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(3) - 4.6357936f) < std::numeric_limits::epsilon()); + REQUIRE(equivalentDiameters.getValue(1) == Approx(6.8127493f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(2) == Approx(5.0081901f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(3) == Approx(4.6357936f).epsilon(k_RelativeTolerance)); } DataStructure CreateRectGridDataStructure() @@ -268,14 +273,14 @@ void ValidateRectGridDataStructure(const DataStructure& dataStructure) REQUIRE(numElements.getValue(3) == 10); // volumes: 0.0 2362.434 352.462 15.104 const auto& volumes = dataStructure.getDataRefAs(k_VolumesPath); - REQUIRE(std::abs(volumes.getValue(1) - 2362.43384f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(volumes.getValue(2) - 352.461884f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(volumes.getValue(3) - 15.1039925f) < std::numeric_limits::epsilon()); + REQUIRE(volumes.getValue(1) == Approx(2362.43384f).epsilon(k_RelativeTolerance)); + REQUIRE(volumes.getValue(2) == Approx(352.461884f).epsilon(k_RelativeTolerance)); + REQUIRE(volumes.getValue(3) == Approx(15.1039925f).epsilon(k_RelativeTolerance)); // eqDiameters: 0.0 16.5242 8.76404 3.06689 const auto& equivalentDiameters = dataStructure.getDataRefAs(k_EquivalentDiametersPath); - REQUIRE(std::abs(equivalentDiameters.getValue(1) - 16.5241966f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(2) - 8.7640428f) < std::numeric_limits::epsilon()); - REQUIRE(std::abs(equivalentDiameters.getValue(3) - 3.0668866f) < std::numeric_limits::epsilon()); + REQUIRE(equivalentDiameters.getValue(1) == Approx(16.5241966f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(2) == Approx(8.7640428f).epsilon(k_RelativeTolerance)); + REQUIRE(equivalentDiameters.getValue(3) == Approx(3.0668866f).epsilon(k_RelativeTolerance)); } } // namespace Test From d9ff3d3bcc58fb04b2f0efe6c1a10f5d8b627764 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Thu, 16 Jul 2026 16:18:05 -0400 Subject: [PATCH 7/9] DOC: Correct ComputeFeatureSizes V&V claims against verified sources Verified every claim in the V&V report and deviations file against the current algorithm/test sources and the stock 6.5.171 FindSizes source (pre-proof-patch commit). Corrections: * Attribute the ImageGeom port to FindSizes::findSizesImage() (execute() only dispatches to it) * Document two previously unlisted port-time behavior changes: the overflow guard tightened from 2^53 to INT32_MAX (legacy silently overflowed int32 NumElements between 2^31 and 2^53; RectGrid path had no guard), and the geometry scope narrowed to Image/RectGrid * Rewrite the ImageGeom precision note from source: 3D Volumes are bit-identical to legacy (same roundings, same order); 2D Areas can differ by <=1 ULP because SIMPLNX evaluates the spacing product in float64 while legacy rounds it to float32 first; ESD/ECD evaluation differences are covered by D2 * Update stale Kahan line citations in D1 (274-283 -> 293-305, 341-358 -> 368-385) * Oracle table: 2D voxel area derivation no longer shows the excluded flat-dimension spacing in the product * Record the new relative-1e-6 test tolerance in the second-engineer review note (was float32 epsilon) Signed-off-by: Michael Jackson --- .../SimplnxCore/vv/ComputeFeatureSizesFilter.md | 10 ++++++---- .../vv/deviations/ComputeFeatureSizesFilter.md | 14 +++++++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md index 08226b711c..7e7f5cd4b1 100644 --- a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md @@ -13,7 +13,7 @@ | Aspect | State | |---|---| -| Algorithm relationship | **Port** of both `FindSizes::execute()` (ImageGeom) and `FindSizes::findSizesUnstructured()` (RectGridGeom) | +| Algorithm relationship | **Port** of both `FindSizes::findSizesImage()` (ImageGeom) and `FindSizes::findSizesUnstructured()` (RectGridGeom) | | Oracle | **Class 1 (Analytical)** — all test data inlined, hand-derived | | Code paths | **14 of 19** exercised; 5 gaps (cancel ×2, INT32_MAX overflow ×2, other-geom no-op ×1) | | Tests | **10 TEST_CASEs** — all pass | @@ -29,13 +29,15 @@ **Port** of `FindSizes` (both ImageGeom and RectGridGeom paths). -- **ImageGeom** (`ProcessImageGeom`): voxel count × voxel volume/area → ESD/ECD. Identical to `FindSizes::execute()`. Formulas: `ESD = 2·∛(V / (4π/3))`; `ECD = 2·√(A / π)`. +- **ImageGeom** (`ProcessImageGeom`): voxel count × voxel volume/area → ESD/ECD. Port of `FindSizes::findSizesImage()` (legacy `execute()` dispatches to it via `findSizes()`). Formulas: `ESD = 2·∛(V / (4π/3))`; `ECD = 2·√(A / π)`. - **RectGridGeom** (`ProcessRectGridGeom`): per-cell `ΔxΔyΔz` sizes, summed per feature. Port of `FindSizes::findSizesUnstructured()`. SIMPLNX departs in precision: float32 element sizes are promoted to float64 and Kahan summation is applied at two levels (per-thread and post-reduction). See deviation `D1`. Port-time additions not in DREAM3D 6.5.171: - **Parallel execution** — `ParallelDataAlgorithm` + `tbb::combinable`; legacy was serial. - **Execute-time FeatureId bounds check** — `ValidateFeatureIdsToFeatureAttributeMatrixIndexing`; legacy accessed out-of-bounds silently. - **Preflight dimension guard** — rejects ImageGeom with two or more dimensions equal to 1. This is because a single empty dimension converts the calculation from Volume to Area and "1D" Images would not make sense to get an Area formula. +- **Tighter overflow guard** — SIMPLNX errors when a feature exceeds `INT32_MAX` voxels on both geometry paths. Legacy `findSizesImage` errored only above 2⁵³ (so counts between 2³¹ and 2⁵³ silently overflowed the int32 `NumElements`), and legacy `findSizesUnstructured` had no guard at all. +- **Narrower geometry scope** — the `GeometrySelectionParameter` allows only Image and RectGrid. Legacy `FindSizes` routed every non-image geometry (vertex/edge/triangle/quad/tet) through `findSizesUnstructured`; in SIMPLNX those are served by dedicated filters (e.g. `ComputeTriangleGeomVolumesFilter`). ## Oracle @@ -43,13 +45,13 @@ Port-time additions not in DREAM3D 6.5.171: | Fixture | Geometry | Derived quantity | |---|---|---| -| `Create2DImageDataStructure()` | 5×5×1 ImageGeom, spacing 20.2×0.1×1.0 | `voxelArea = 20.2 × 0.1 × 1.0 = 2.02`; areas = count × 2.02; ECDs from circle formula | +| `Create2DImageDataStructure()` | 5×5×1 ImageGeom, spacing 20.2×0.1×1.0 | `voxelArea = 20.2 × 0.1 = 2.02` (flat-Z spacing excluded); areas = count × 2.02; ECDs from circle formula | | `Create3DImageDataStructure()` | 5×5×5 ImageGeom, spacing 1.2×0.9×2.1 | `voxelVol = 1.2 × 0.9 × 2.1 = 2.268`; volumes = count × 2.268; ESDs from sphere formula | | `CreateRectGridDataStructure()` | 4×4×4 non-uniform RectGrid | Per-cell `ΔxΔyΔz` summed per feature; step-by-step trace in provenance sidecar | Negative tests use degenerate inputs (out-of-bounds FeatureId; degenerate dims) and assert an error result is returned. -Second-engineer review pending: verify hand-derivations for all three fixtures and the tolerance choice (`std::numeric_limits::epsilon()`). +Second-engineer review pending: verify hand-derivations for all three fixtures and the tolerance choice (relative `1e-6` via Catch2 `Approx` — a few float32 ULPs of slack so the pins survive platform and TBB reduction-order differences). ## Code path coverage diff --git a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md index 0e2bea962c..1c526a5c1a 100644 --- a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md @@ -8,15 +8,19 @@ Entries are referenced by stable ID (`ComputeFeatureSizes-D`) from the V&V re ## ImageGeom path — no deviations -Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `FindSizes::execute()` confirmed identical algorithmic structure: +Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `FindSizes::findSizesImage()` confirmed identical algorithmic structure: -- Per-feature voxel count: identical parallel accumulation logic (serial in legacy). +- Per-feature voxel count: same accumulation logic (SIMPLNX parallelizes it via TBB; legacy is serial — the reduction is exact integer addition either way). - 3D volume formula: `volume = voxelCount × voxelVolume`; ESD: `2·∛(volume / (4π/3))` — both use the standard sphere formula. - 2D area formula (exactly one dim == 1): `area = voxelCount × (product of the two NON-flat spacings)`; ECD: `2·√(area / π)` — both use the standard circle formula, and both exclude the flat dimension's spacing. > **Corrected this V&V cycle (was a latent SIMPLNX bug, now no deviation):** `ProcessImageGeom` previously multiplied **all three** spacings for the 2D voxel area (`spacing[0]·spacing[1]·spacing[2]`), which is a volume, not an area. It diverged from DREAM3D 6.5.171 (which uses only the two non-flat resolutions) whenever the flat dimension's spacing was not 1.0. The prior 2D unit fixture used a flat-dimension spacing of exactly 1.0, so the defect was invisible. The formula now excludes the flat dimension's spacing, restoring parity with legacy — hence this remains a *no-deviation* entry. Pinned by the `2D area excludes the flat-dimension spacing` characterization test (flat X/Y/Z, non-unit flat spacing). -**Precision non-deviation (not flagged):** SIMPLNX promotes `voxelVolume` to `float64` and evaluates `cbrt` / `sqrt` in `float64` before casting to `float32` for storage. The legacy filter likely used `float32` throughout. The difference is within ~1 ULP of `float32` for typical EBSD spacings and is non-material for downstream morphological statistics. +**Precision notes (verified against 6.5.171 source, not flagged as deviations):** + +- *3D `Volumes`: bit-identical.* Both implementations compute the voxel volume as a float32 product of the three float32 spacings (SIMPLNX widens the float32 result to float64 afterward; legacy casts it to double) and multiply by the voxel count in double before storing as float32 — the same roundings in the same order. +- *2D `Areas`: potential ≤1 float32 ULP difference.* SIMPLNX evaluates the two-non-flat-spacing product in float64 (exact for two float32 factors); legacy `findSizesImage` rounds the product to float32 first (`res_scalar = xRes * yRes`). When that product is inexact in float32, the stored per-feature area can differ by one ULP. Not observable in the current fixtures and non-material for downstream statistics; no A/B run has been performed for the ImageGeom path. +- *ESD/ECD evaluation:* legacy uses float32 `powf`/`sqrtf` on the float32-rounded stored value, SIMPLNX uses float64 `std::cbrt`/`std::sqrt` on the unrounded float64 value — this is the same evaluation-precision pattern documented (and A/B-verified on RectGrid) as `ComputeFeatureSizes-D2`. --- @@ -36,8 +40,8 @@ Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `Find 1. **float32 → float64 promotion.** Element sizes are stored as `float32` (the output of `findElementSizes`). The legacy `findSizesUnstructured` accumulated these directly in `float32`, so each per-cell addition carried a relative error of ~`ε_float32 ≈ 1.2×10⁻⁷`. SIMPLNX promotes each element size to `float64` before accumulation (`static_cast(m_ElemSizes.getValue(voxelIdx))`), reducing the per-operation rounding error to `ε_float64 ≈ 2.2×10⁻¹⁶`. 2. **Kahan summation.** SIMPLNX applies Kahan compensated summation at two levels: - - *Per-thread, per-cell* in `RectGridSummationImpl::convert()` (lines 274–283): standard Kahan with `y = elemSize - c`, `t = sum + y`, `c′ = (t - sum) - y`. For a feature spanning N cells, naive float64 accumulation still has O(N · ε_float64) error; Kahan reduces this to O(ε_float64) by carrying a compensation term `c` that recovers the low-order bits lost in each `sum + y` operation. - - *Post-reduction, per thread-local vector* in `threadLocalVolumes.combine_each()` (lines 341–358): the same Kahan scheme is applied when combining the TBB thread-local partial sums into the final per-feature volume. + - *Per-thread, per-cell* in `RectGridSummationImpl::convert()` (lines 293–305): standard Kahan with `y = elemSize - c`, `t = sum + y`, `c′ = (t - sum) - y`. For a feature spanning N cells, naive float64 accumulation still has O(N · ε_float64) error; Kahan reduces this to O(ε_float64) by carrying a compensation term `c` that recovers the low-order bits lost in each `sum + y` operation. (Note the compensator is a local that resets on each TBB body invocation — see the V&V report's D1 summary.) + - *Post-reduction, per thread-local vector* in `threadLocalVolumes.combine_each()` (lines 368–385): the same Kahan scheme is applied when combining the TBB thread-local partial sums into the final per-feature volume. For a non-uniform rectilinear grid where cell volumes span several orders of magnitude (e.g., a grid with fine near-surface resolution and coarse interior), naive summation of N cells can lose ~`log₂(V_max / V_min)` bits of precision in the smaller cell contributions. Kahan summation recovers those bits regardless of N or the dynamic range of cell volumes. From ec9086d2c4ed478ff6373550b9ec52fd8ba34e33 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Fri, 17 Jul 2026 11:45:09 -0400 Subject: [PATCH 8/9] Revert Code change to the PR #1590 conventions for 2D Image Geometries. Added a TEST_CASE that will exercise the issue. This test will fail and force the CI to be RED until the issue is figured out. --- .../Algorithms/ComputeFeatureSizes.cpp | 25 +++++------------- .../test/ComputeFeatureSizesTest.cpp | 26 ++++++++++++------- .../vv/ComputeFeatureSizesFilter.md | 10 +++---- .../deviations/ComputeFeatureSizesFilter.md | 9 +++---- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp index 0fa5a778d6..19202699d1 100644 --- a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp +++ b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ComputeFeatureSizes.cpp @@ -148,24 +148,13 @@ Result<> ProcessImageGeom(ImageGeom& imageGeom, Float32AbstractDataStore& volume * For these two cases the following code would BREAK, so do not enable. **/ - // Calculate the area of a single voxel from the two NON-flat dimensions only. Exactly one dimension - // is flat here (preflight rejects more than one empty dimension), so its spacing must be excluded -- - // including it would compute a volume, not an area, and would diverge from DREAM3D 6.5.171 whenever - // the flat dimension's spacing != 1. This matches legacy FindSizes::findSizesImage, which uses only - // the two non-flat resolutions. - float64 voxelArea = 1.0; - if(xDimSize != 1) - { - voxelArea *= static_cast(spacing[0]); - } - if(yDimSize != 1) - { - voxelArea *= static_cast(spacing[1]); - } - if(zDimSize != 1) - { - voxelArea *= static_cast(spacing[2]); - } + // OPEN DESIGN QUESTION: this includes the flat dimension's spacing, matching the PR #1590 + // "slab" convention used by ImageGeom::findElementSizes, but it diverges from legacy DREAM3D + // 6.5.171 (FindSizes::findSizesImage uses only the two non-flat resolutions) whenever the flat + // dimension's spacing != 1. See the "[2DFlatSpacing]" test case, which characterizes the + // divergence, and the ComputeFeatureSizesFilter V&V deviations entry. + // Calculate the area of a single voxel + const float64 voxelArea = static_cast(spacing[0]) * static_cast(spacing[1]) * static_cast(spacing[2]); msgHelper.sendMessage("Feature Level: Storing Voxel Counts and Calculating Area and ECD..."); // Process each feature storing feature voxel counts, areas, and equivalent circular diameter diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index c7a7fe4e16..ca535b4783 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -324,13 +324,19 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: Valid: Image 2D", "[SimplnxCore][Co UnitTest::CheckArraysInheritTupleDims(dataStructure); } -// Characterization test for the 2D area formula: the flat dimension's spacing must be EXCLUDED so a -// 2D area is a true area, not a volume. The existing "Valid: Image 2D" fixture uses a flat-dimension -// spacing of 1.0, which hides an all-three-spacings bug; here the flat dimension's spacing is -// deliberately non-unit (5.0). If the flat spacing were (incorrectly) included, every area would be -// 5x too large. All three flat orientations are checked so the fix cannot be an axis-specific special -// case. This matches DREAM3D 6.5.171, which uses only the two non-flat resolutions. -TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension spacing", "[SimplnxCore][ComputeFeatureSizes]") +// Characterization test for an OPEN BUG / pending design decision on the 2D area formula. +// +// The assertions below encode the legacy DREAM3D 6.5.171 semantics (FindSizes::findSizesImage): +// a 2D area is the product of the two NON-flat spacings only, so the single feature's area is +// 4 voxels * (2.0 * 3.0) = 24.0. The current implementation follows the PR #1590 "slab" convention +// (matching ImageGeom::findElementSizes) and multiplies ALL THREE spacings, yielding 120.0 whenever +// the flat dimension's spacing != 1. The existing "Valid: Image 2D" fixture uses a flat-dimension +// spacing of exactly 1.0, which is why it cannot see the divergence; here the flat spacing is +// deliberately 5.0, and all three flat orientations are GENERATEd so no axis-specific special case +// can satisfy it. +// +// While the #1590 convention is in place, this test fails. +TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension spacing", "[SimplnxCore][ComputeFeatureSizes][2DFlatSpacing]") { auto [label, dims, spacing] = GENERATE(std::make_tuple("flat Z", SizeVec3{2, 2, 1}, FloatVec3{2.0f, 3.0f, 5.0f}), std::make_tuple("flat X", SizeVec3{1, 2, 2}, FloatVec3{5.0f, 2.0f, 3.0f}), std::make_tuple("flat Y", SizeVec3{2, 1, 2}, FloatVec3{2.0f, 5.0f, 3.0f})); @@ -338,7 +344,7 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension DYNAMIC_SECTION(label) { // Four cells, all feature 1 (feature 0 unused). Non-flat spacings are always 2.0 and 3.0, so the - // per-voxel area is 6.0 and the single feature's area is 4 * 6.0 = 24.0 regardless of which axis is + // per-voxel area is 6.0, and the single feature's area is 4 * 6.0 = 24.0 regardless of which axis is // flat. The flat dimension's spacing is 5.0 and must NOT appear in the product. DataStructure dataStructure; auto* imageGeom = ImageGeom::Create(dataStructure, Test::k_ImageGeomName); @@ -369,7 +375,9 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result); const auto& areas = dataStructure.getDataRefAs(Test::k_VolumesPath); - REQUIRE(areas.getValue(1) == Approx(24.0f)); // 4 voxels * (2.0 * 3.0); NOT 4 * (2.0 * 3.0 * 5.0) = 120 + // Legacy 6.5.171 expectation: 4 voxels * (2.0 * 3.0) = 24.0. The current #1590 slab convention + // produces 4 * (2.0 * 3.0 * 5.0) = 120.0, so this assertion fails (expected, see [!shouldfail]). + REQUIRE(areas.getValue(1) == Approx(24.0f)); UnitTest::CheckArraysInheritTupleDims(dataStructure); } diff --git a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md index 7e7f5cd4b1..e86199d600 100644 --- a/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/ComputeFeatureSizesFilter.md @@ -16,14 +16,14 @@ | Algorithm relationship | **Port** of both `FindSizes::findSizesImage()` (ImageGeom) and `FindSizes::findSizesUnstructured()` (RectGridGeom) | | Oracle | **Class 1 (Analytical)** — all test data inlined, hand-derived | | Code paths | **14 of 19** exercised; 5 gaps (cancel ×2, INT32_MAX overflow ×2, other-geom no-op ×1) | -| Tests | **10 TEST_CASEs** — all pass | +| Tests | **10 TEST_CASEs** — 9 pass; 1 (`[!shouldfail]`) fails-as-expected, pinning the open 2D divergence | | External archive | None | | Deviations | **2 active**, both A/B-verified 2026-06-27: `ComputeFeatureSizes-D1` (float64+Kahan vs naive summation → `Volumes`), `ComputeFeatureSizes-D2` (float64 `std::cbrt` vs float32 `powf` → `EquivalentDiameters`) — see deviations file | -| Open bugs | **None.** `Bug-1` (2D area formula multiplied all 3 spacings instead of the 2 non-flat spacings) was **fixed this cycle** and pinned by the `2D area excludes the flat-dimension spacing` characterization test; the ImageGeom 2D path now matches legacy (no deviation). | +| Open bugs | **1 open, pending design decision** (`Bug-1`): the 2D area formula multiplies all 3 spacings (PR #1590 slab convention, consistent with `ImageGeom::findElementSizes`) and diverges from legacy 6.5.171 (2 non-flat spacings) when the flat dimension's spacing ≠ 1. A legacy-matching fix was applied then deliberately reverted pending a team decision on the NX-wide 2D convention (see PR #1638 discussion and the deviations file's OPEN entry). | ## Summary -`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. Two precision deviations (`D1`, `D2`) — both confirmed by a direct A/B run against DREAM3D 6.5.171 — are documented below. A latent 2D-area bug (`Bug-1`: the flat dimension's spacing was included in the per-voxel area) was found during this V&V and **fixed**; the 2D path now matches legacy and is pinned by a dedicated characterization test with a non-unit flat-dimension spacing. +`ComputeFeatureSizesFilter` produces three arrays per feature: `NumElements` (voxel count, `int32`), `Volumes`/`Areas` (float32), and `EquivalentDiameters` (ESD or ECD, float32). For ImageGeom it uses voxel count × voxel size; for RectGridGeom it sums per-cell element sizes per feature. Both paths are parallelized via `ParallelDataAlgorithm` + TBB `tbb::combinable`. All tests use Class 1 oracles (hand-constructed fixtures with first-principles expected values). Source-inspection confirms both geometry paths are ports of legacy `FindSizes`. Two precision deviations (`D1`, `D2`) — both confirmed by a direct A/B run against DREAM3D 6.5.171 — are documented below. A 2D-area divergence from legacy (`Bug-1`: the flat dimension's spacing is included in the per-voxel area, per the PR #1590 slab convention) was found during this V&V; a legacy-matching fix was applied and then deliberately reverted because it conflicted with the NX-wide 2D standardization from PR #1590 without a team decision. The divergence remains **open** and is pinned by a dedicated `[!shouldfail]` characterization test with a non-unit flat-dimension spacing; see the deviations file's OPEN entry for the two resolution options. ## Algorithm Relationship @@ -84,7 +84,7 @@ Second-engineer review pending: verify hand-derivations for all three fixtures a | Test case | Status | Notes | |---|---|---| | `Valid: Image 2D` | kept | Class 1; 5×5×1, spacing 20.2×0.1×1.0 (flat-Z spacing 1.0); SaveElementSizes=false | -| `2D area excludes the flat-dimension spacing` | new-for-V&V | Class 1 characterization pin for the Bug-1 fix. 2×2 slab, non-flat spacings 2.0×3.0, flat-dimension spacing **5.0**; asserts area == 24.0 (not 120.0). `GENERATE`s all three flat orientations (X/Y/Z) so the flat spacing is proven excluded regardless of axis. | +| `2D area excludes the flat-dimension spacing` | new-for-V&V | Class 1 characterization pin for the **open** Bug-1 divergence, tagged `[!shouldfail]`. 2×2 slab, non-flat spacings 2.0×3.0, flat-dimension spacing **5.0**; asserts the legacy value area == 24.0, while the current #1590 slab convention produces 120.0 — so it fails-as-expected until the 2D convention decision lands. `GENERATE`s all three flat orientations (X/Y/Z) so no axis-specific special case can satisfy it. | | `Valid: Image 2D with Element Sizes` | kept | Same fixture as `Valid: Image 2D`; SaveElementSizes=true | | `Valid: Image Stack 3D` | kept | Class 1; 5×5×5, spacing 1.2×0.9×2.1; SaveElementSizes=false | | `Valid: Image Stack 3D with Element Size` | kept | Same fixture; SaveElementSizes=true | @@ -95,7 +95,7 @@ Second-engineer review pending: verify hand-derivations for all three fixtures a | `SIMPL Backwards Compatibility` | kept | `DYNAMIC_SECTION` over SIMPL 6.4 + 6.5; validates UUID + arg-key + param-value decoding | | `Legacy: Small IN100 Test` | retired | Real-data comparison against legacy-produced `6_6_stats_test_v2.dream3d` arrays. Retired because it was a legacy-output regression check (not an independent oracle) and the D1/D2 precision deviations intentionally change those exact values; the RectGrid A/B (deviations file) now provides the legacy comparison and the inline Class 1 fixtures provide the correctness oracle. | -All 10 TEST_CASEs pass. +9 of 10 TEST_CASEs pass; the `[!shouldfail]` characterization test fails-as-expected (ctest green) while Bug-1 is unresolved. ## Exemplar archive diff --git a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md index 1c526a5c1a..e8234ddd29 100644 --- a/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md +++ b/src/Plugins/SimplnxCore/vv/deviations/ComputeFeatureSizesFilter.md @@ -6,20 +6,19 @@ Entries are referenced by stable ID (`ComputeFeatureSizes-D`) from the V&V re --- -## ImageGeom path — no deviations +## ImageGeom path — one open divergence (2D area, pending design decision) -Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `FindSizes::findSizesImage()` confirmed identical algorithmic structure: +Source-inspection comparison of `ProcessImageGeom` against DREAM3D 6.5.171 `FindSizes::findSizesImage()` confirmed identical algorithmic structure for 3D and for voxel counting: - Per-feature voxel count: same accumulation logic (SIMPLNX parallelizes it via TBB; legacy is serial — the reduction is exact integer addition either way). - 3D volume formula: `volume = voxelCount × voxelVolume`; ESD: `2·∛(volume / (4π/3))` — both use the standard sphere formula. -- 2D area formula (exactly one dim == 1): `area = voxelCount × (product of the two NON-flat spacings)`; ECD: `2·√(area / π)` — both use the standard circle formula, and both exclude the flat dimension's spacing. -> **Corrected this V&V cycle (was a latent SIMPLNX bug, now no deviation):** `ProcessImageGeom` previously multiplied **all three** spacings for the 2D voxel area (`spacing[0]·spacing[1]·spacing[2]`), which is a volume, not an area. It diverged from DREAM3D 6.5.171 (which uses only the two non-flat resolutions) whenever the flat dimension's spacing was not 1.0. The prior 2D unit fixture used a flat-dimension spacing of exactly 1.0, so the defect was invisible. The formula now excludes the flat dimension's spacing, restoring parity with legacy — hence this remains a *no-deviation* entry. Pinned by the `2D area excludes the flat-dimension spacing` characterization test (flat X/Y/Z, non-unit flat spacing). +> **OPEN — 2D area formula (exactly one dim == 1), pending design decision:** SIMPLNX multiplies **all three** spacings for the 2D per-voxel size (`spacing[0]·spacing[1]·spacing[2]`), following the PR #1590 "slab" convention that standardized 2D Image Geometry handling NX-wide (matching `ImageGeom::findElementSizes`). Legacy 6.5.171 uses only the **two non-flat** resolutions (a true area). The outputs therefore diverge whenever the flat dimension's spacing is not 1.0: `Volumes` scale by the flat spacing and `EquivalentDiameters` (circle-ECD from that value) by its square root. Note the current 2D branch feeds this three-spacing product into the circle-ECD formula `2·√(x/π)`, which is dimensionally inconsistent (a volume into an area formula). A legacy-matching fix was applied and then deliberately reverted (see PR #1638 discussion) because it conflicted with the #1590 convention without a team decision; the divergence is characterized by the `2D area excludes the flat-dimension spacing` test (tagged `[!shouldfail]`, flat X/Y/Z orientations, flat spacing 5.0), which asserts the legacy value and fails-as-expected until the convention is settled. Resolution options: (A) adopt legacy true-2D semantics here and in `findElementSizes`, or (B) keep the slab convention, switch the 2D branch to ESD for dimensional coherence, and promote this entry to a numbered deviation. **Precision notes (verified against 6.5.171 source, not flagged as deviations):** - *3D `Volumes`: bit-identical.* Both implementations compute the voxel volume as a float32 product of the three float32 spacings (SIMPLNX widens the float32 result to float64 afterward; legacy casts it to double) and multiply by the voxel count in double before storing as float32 — the same roundings in the same order. -- *2D `Areas`: potential ≤1 float32 ULP difference.* SIMPLNX evaluates the two-non-flat-spacing product in float64 (exact for two float32 factors); legacy `findSizesImage` rounds the product to float32 first (`res_scalar = xRes * yRes`). When that product is inexact in float32, the stored per-feature area can differ by one ULP. Not observable in the current fixtures and non-material for downstream statistics; no A/B run has been performed for the ImageGeom path. +- *2D `Areas`: potential ≤1 float32 ULP difference even when the flat spacing is 1.0.* Setting aside the open three-vs-two-spacing divergence above (which dominates whenever the flat spacing ≠ 1.0), the evaluation precision also differs: SIMPLNX computes the spacing product in float64; legacy `findSizesImage` rounds the two-resolution product to float32 first (`res_scalar = xRes * yRes`). When that product is inexact in float32, the stored per-feature area can differ by one ULP. Not observable in the current fixtures and non-material for downstream statistics; no A/B run has been performed for the ImageGeom path. - *ESD/ECD evaluation:* legacy uses float32 `powf`/`sqrtf` on the float32-rounded stored value, SIMPLNX uses float64 `std::cbrt`/`std::sqrt` on the unrounded float64 value — this is the same evaluation-precision pattern documented (and A/B-verified on RectGrid) as `ComputeFeatureSizes-D2`. --- From 5ca3f12f465dded9506fb383a4ba8fcfb6073738 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Wed, 22 Jul 2026 22:00:49 -0400 Subject: [PATCH 9/9] TEST: Add missing [!shouldfail] tag to 2D flat-spacing characterization test The revert of the 2D area fix (f03c871ae) documented the characterization test as a Catch2 [!shouldfail] expected failure -- the V&V report, the deviations file, and the test's own comments all reference the tag -- but the tag was never added to the TEST_CASE tag string, so the test hard-failed CI on all platforms. * Add [!shouldfail] so the open 2D-area divergence stays pinned while CI stays green; the test flips to a hard failure the moment the divergence is resolved, forcing it to be updated with the design decision * Expand the leading comment to state that behavior explicitly Signed-off-by: Michael Jackson --- src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp index ca535b4783..f24fe4b4a0 100644 --- a/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp +++ b/src/Plugins/SimplnxCore/test/ComputeFeatureSizesTest.cpp @@ -335,8 +335,10 @@ TEST_CASE("SimplnxCore::ComputeFeatureSizes: Valid: Image 2D", "[SimplnxCore][Co // deliberately 5.0, and all three flat orientations are GENERATEd so no axis-specific special case // can satisfy it. // -// While the #1590 convention is in place, this test fails. -TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension spacing", "[SimplnxCore][ComputeFeatureSizes][2DFlatSpacing]") +// While the #1590 convention is in place, the assertions below fail; the [!shouldfail] tag makes +// Catch2 report that as an expected failure (so CI stays green) and flips to a hard failure the +// moment the divergence is resolved, forcing this test to be updated alongside the design decision. +TEST_CASE("SimplnxCore::ComputeFeatureSizes: 2D area excludes the flat-dimension spacing", "[SimplnxCore][ComputeFeatureSizes][2DFlatSpacing][!shouldfail]") { auto [label, dims, spacing] = GENERATE(std::make_tuple("flat Z", SizeVec3{2, 2, 1}, FloatVec3{2.0f, 3.0f, 5.0f}), std::make_tuple("flat X", SizeVec3{1, 2, 2}, FloatVec3{5.0f, 2.0f, 3.0f}), std::make_tuple("flat Y", SizeVec3{2, 1, 2}, FloatVec3{2.0f, 5.0f, 3.0f}));