BUG: C20: Parameters not wired to filter behavior in 22 filters #1879

Summary

In many filters, a parameter is not connected correctly to what the algorithm does. The parameter is ignored, validated while it is hidden and unused, not validated while the algorithm still reads it, or mapped from the wrong legacy key. Related problems: an index from file or Ensemble data selects an orientation-ops object with no range check, and a Feature id permutation is applied to only some id-indexed arrays.

As of develop 419f9db (2026-10-21) there are 26 open findings in 22 filters: 5 CRASH, 10 WRONG_DATA and 11 MINOR. Each finding is a candidate. It was found and re-checked by reading code, not by running it.

Mechanism

Parameter linking (linkParameters), preflight validation and the algorithm's InputValues are written separately. Nothing checks that every parameter is read, or that validation follows the link state. This class merges four sub-patterns:

• Parameter value disconnected from behavior. A parameter is ignored (seed, max iterations, stripe width, data format, phase-by-phase, Cell Data Attribute Matrix). Or preflight validates a value that is hidden and unused. Or preflight skips a linked parameter that the algorithm always reads. Or a hidden default changes the result.
• Orientation-ops index used unchecked. A value from determineOrientationOpsIndex(), a space group number or a CrystalStructures value (999 means unknown) indexes ebsdlib::LaueOps::GetAllOrientationOps(), or the pointer from GetOrientationOpsFromSpaceGroupNumber() is dereferenced with no null check. No helper at 419f9db handles an unknown symmetry.
• FromSIMPLJson maps legacy keys wrongly. A converter reads the wrong legacy key, or one converter serves two legacy UUIDs whose behavior differs. A converted pipeline then runs a different operation.
• Feature id permutation applied to only some arrays. A random permutation of Feature ids is applied to the cell ids but not to every Feature Attribute Matrix array, or it is applied with single swaps instead of full cycles. Feature Attribute Matrix rows then do not match the ids.

Example (AddBadDataFilter::executeImpl). When Use Seed for Random Generation is off, the filter makes a clock seed and stores it in the seed DataArray. Then it passes the parameter seed, not the clock seed, to the algorithm. Every run uses the same seed, and the stored seed does not reproduce the output:

  auto seed = filterArgs.value<uint64>(k_SeedValue_Key);
  if(!filterArgs.value<bool>(k_UseSeed_Key))
  {
    seed = static_cast<std::mt19937_64::result_type>(std::chrono::steady_clock::now().time_since_epoch().count());
  }

  // Store Seed Value in Top Level Array
  dataStructure.getDataRefAs<UInt64Array>(DataPath({filterArgs.value<std::string>(k_SeedArrayName_Key)}))[0] = seed;
  ...
  inputValues.SeedValue = filterArgs.value<uint64>(k_SeedValue_Key);

Source: src/Plugins/SimplnxCore/src/SimplnxCore/Filters/AddBadDataFilter.cpp#L154-L171

Example (orientation-ops index). GeneratePreflightPhaseInformation runs in preflight. An unknown symmetry makes determineOrientationOpsIndex() return 999, but laueOps has about 12 entries:

      preflightUpdatedValues.push_back({fmt::format("{}: ", phaseInfo->getPhaseIndex()), fmt::format("Material Name: {}    |    Formula: {}    |    Crystal Symmetry: {}", phaseInfo->getMaterialName(),
                                                                                                     phaseInfo->getFormula(), laueOps[phaseInfo->determineOrientationOpsIndex()]->getSymmetryName())});

Source: src/Plugins/OrientationAnalysis/src/OrientationAnalysis/utilities/EbsdReaderUtilities.hpp#L297-L298

No filter of this class is fixed on develop or in an open pull request yet.

Rule

General rules (decided for all classes):

• Validation follows the parameter link state exactly. Validate a value only when the algorithm uses it. Always validate a value that the algorithm always reads; if needed, remove the link.
• Checks belong in preflight unless they need DataArray values. Remove execute-time checks that a new preflight check makes unreachable.

Per sub-pattern:

• Disconnected parameter: pass every parameter into InputValues and use it, or remove it.
• Orientation-ops index: add one checked accessor (index < GetAllOrientationOps().size(), pointer not null) and use it in all EBSD readers and GBCD/GMT writers.
• FromSIMPLJson: give each legacy UUID its own converter that sets mode keys explicitly.
• Permutation: build one permutation and apply it to every id-indexed array (cell ids and all Feature Attribute Matrix arrays), through a temporary copy or full cycle-following.

Decision needed:

• For a parameter that has no effect (Hatch Length / StripeWidth, Face Attribute Matrix, Cell Data, Data Format): (a) implement the behavior, or (b) remove the parameter with a filter version bump and a FromSIMPLJson/pipeline-compatibility note.
• For an unknown symmetry in a reader's preflight phase summary: (a) show "Unknown" and continue, or (b) return a preflight error. A writer that needs the operations (WriteGBCDGMTFileFilter) must return a preflight error.

How to fix

1. Read the filter's parameters(), linkParameters calls, preflightImpl() and executeImpl() together. List each parameter, its link state, where preflight validates it and where the algorithm reads it.
2. Disconnected parameter: copy the value into InputValues and use it in the algorithm. For seeds, pass the seed that is stored in the seed DataArray (for example inputValues.SeedValue = seed;), and seed the generator from m_InputValues->Seed (MergeTwins).
3. Hidden or unused parameter: put its validation inside the same condition that the link uses (for example validate Reference Slice only when Use Reference Slice is on).
4. Always-read parameter: remove the link, or always validate it (FeatureFaceCurvatureFilter Face Normals).
5. Orientation-ops index: write one checked accessor in OrientationAnalysis (no such helper exists at 419f9db; the only size checks are local, for example ComputeKernelAvgMisorientations.cpp uses GetAllOrientationOps().size()). Use it at all four branches of GeneratePreflightPhaseInformation (EbsdReaderUtilities.hpp lines 297-298, 308-309, 319-320 and 326-327) and in WriteGBCDGMTFile.cpp.
6. FromSIMPLJson: read the correct legacy key. Split a shared converter per legacy UUID. Set every mode key explicitly instead of keeping defaults.
7. Permutation: apply one permutation to all id-indexed arrays. In ClusterUtilities::RandomizeFeatureIds (src/simplnx/Utilities/ClusteringUtilities.cpp), follow each cycle fully, or copy through a temporary per array. In MergeTwins and SharedFeatureFace, randomize before the outputs are written, or remap every related array with the same permutation.
8. New error codes: use the filter's existing error-code range. Codes are more negative than -10000, and each filter uses one range of 100 codes. Check uniqueness with git grep -nE -- "-NNNN([^0-9]|$)" (not \b) on develop and on open pull request refs.
9. If you remove a parameter, bump the filter version, and update FromSIMPLJson and the test pipelines that set it.

How to test

Write a unit test that fails on develop first. Then fix the code.

• Ignored parameter: run the filter twice with two values of the parameter. Assert that the output changes as an independent expected result says. For seeds: with Use Seed off, re-run with the stored seed and Use Seed on, and assert identical output.
• Validation and link state: a preflight test with the parameter hidden and an invalid value (must pass), and a preflight test with the parameter used and an invalid value (must fail with the expected code).
• Orientation-ops index: a preflight test with a small test file whose phase symmetry, Laue group or space group is unknown (for example 999 or -1). Assert no crash and the decided output ("Unknown" or the error code).
• FromSIMPLJson: one conversion test per legacy UUID, using a real legacy pipeline JSON. Assert the converted arguments, and for mode keys assert the behavior.
• Permutation: a hand-built Feature Attribute Matrix with known per-Feature values. Use a permutation with a cycle of length 4 or more (for example randomIds = [0,2,3,4,1]). Assert that for each cell, FeatureArray[FeatureIds[cell]] equals the value before randomization. This oracle does not depend on the random numbers.
• If the filter has Direct and Scanline algorithms, test both through the existing algorithm-selection test scope.
• Value checks need an independent expected result (oracle), not the output of the code under test.

Conventions

• One pull request for the whole class. One commit per filter.
• Commit prefix BUG:. Add a Signed-off-by line. Do not add AI attribution.
• Every fix starts with a unit test that fails on develop. If you cannot reproduce a finding, write that in the pull request and on this issue with the reason. Do not force a fix.
• Value checks need an independent expected result (oracle).
• Run ctest serially. Never use -j; the tests depend on each other.
• Format with clang-format 19.1.1 and the repository .clang-format.
• Do not add per-filter documentation sentences about input requirements. The generated parameter table documents them.
• If a fix touches a filter that has a V&V report (src/Plugins/<Plugin>/vv/<Filter>.md), update every file of that filter's V&V package (report, deviations, provenance, test tables). V&V documents reference DREAM3D 6.5.171 only.
• Rebase on develop. Never merge develop into the branch.

Checklist

• CRASH · FeatureFaceCurvatureFilter · CalculateTriangleGroupCurvatures.cpp:221 — With Use Face Normals for Curve Fitting off, Face Normals is hidden and not validated, but the algorithm always calls m_SurfaceMeshFaceNormals->getDataStoreRef(): null dereference. FeatureFaceCurvatureFilter#2
• CRASH · ReadGrainMapper3DFilter · EbsdReaderUtilities.hpp:327 — A phase SpaceGroup of 999 or -1 makes GetOrientationOpsFromSpaceGroupNumber() return null, and preflight dereferences it. ReadGrainMapper3DFilter#8
• CRASH · ReadH5EbsdFilter · EbsdReaderUtilities.hpp:297 — An .h5ebsd phase with an unknown symmetry or Laue group makes determineOrientationOpsIndex() return 999, and preflight indexes laueOps (about 12 entries) out of bounds. Same code at lines 308-309 and 319-320. ReadH5EbsdFilter#5
• CRASH · ReadH5OimDataFilter · EbsdReaderUtilities.hpp:297 — An OIM phase Symmetry that matches no case (for example 0) gives index 999 and an out-of-bounds read in preflight. ReadH5OinaDataFilter (line 309, Laue Group 0 or > 11) and the H5Esprit branch (line 320) do the same. ReadH5OimDataFilter#1
• CRASH · WriteGBCDGMTFileFilter · WriteGBCDGMTFile.cpp:155 — Phase of Interest = 0 (CrystalStructures[0] = 999) indexes past GetAllOrientationOps() and line 157 dereferences garbage. Phase of Interest = -1 gives a misleading -96712. WriteGBCDGMTFileFilter#5
• WRONG_DATA · AddBadDataFilter · AddBadDataFilter.cpp:171 — With Use Seed off, the clock seed is stored in the seed DataArray but the algorithm gets the Seed Value parameter, so every run is identical and the stored seed does not reproduce the output. AddBadDataFilter#1
• WRONG_DATA · ComputeBiasedFeaturesFilter · ComputeBiasedFeatures.cpp:198 — The 2D path never reads Apply Phase by Phase or the Phases DataArray, so the option is ignored without a warning when an Image Geometry dimension is 1. ComputeBiasedFeaturesFilter#3
• WRONG_DATA · CreateAMScanPathsFilter · CreateAMScanPaths.cpp:403 — StripeWidth (Hatch Length) is never read by hatch generation, so Hatch Length 0.5 and 3.0 give the same scan paths, but the documentation shows different outputs. CreateAMScanPathsFilter#1
• WRONG_DATA · ExtractComponentAsArrayFilter · ExtractComponentAsArrayFilter.cpp:179 — FromSIMPLJson keeps Move=false and Remove=false for both legacy UUIDs and drops ReducedArrayArrayName/SaveRemovedComponent, so converted pipelines change the array in place and never create NewArrayArrayName. ExtractComponentAsArrayFilter#2
• WRONG_DATA · HierarchicalSmoothFilter · HierarchicalSmooth.cpp:600 — vs.maxIterations is set but never read; smoothWithType and smooth use the default nIter=53, so Max Bisection Iterations has no effect. (related: BUG: Hierarchical smoothing ignores Max Bisection Iterations #1791) HierarchicalSmoothFilter#2
• WRONG_DATA · MergeTwinsFilter · MergeTwins.cpp:322 — With Randomize Parent Ids on, Cell Parent Ids are written first and then only Feature Parent Ids are permuted, so Cell Parent Ids no longer equal Feature Parent Ids[FeatureIds]. numParents comes from the cells, so a read past randomIds is also possible. MergeTwinsFilter#2
• WRONG_DATA · RandomizeFeatureIdsFilter · ClusteringUtilities.cpp:107 — The visited/swap loop does one swap per pair. For a cycle of length 4 or more (for example randomIds = [0,2,3,4,1]) the Feature Attribute Matrix rows do not match the new Feature Ids. (related: BUG: Randomize Feature Ids misaligns attributes for longer cycles #1785) RandomizeFeatureIdsFilter#1
• WRONG_DATA · ReadH5EbsdFilter · ReadH5EbsdFilter.cpp:371 — FromSIMPLJson sets eulerRepresentation from RefFrameZDir - 1 and ignores AngleRepresentation, so a converted pipeline with Radians data and transformations on gets the wrong degree-to-radian scale. ReadH5EbsdFilter#1
• WRONG_DATA · ReadImageFilter · ReadImageFilter.cpp:249 — is2D = true hides the Z crop controls, but the default keeps cropZ=true and zBoundVoxels={0,0}, so cropping X only on a multi-page TIFF or 3D .nrrd crops it to Z=1. ReadImageFilter#3
• WRONG_DATA · SharedFeatureFaceFilter · SharedFeatureFace.cpp:240 — Randomize Face IDs permutes only the triangle Feature Face Ids after the Face Feature Attribute Matrix labels and counts were written, so row k no longer describes the triangles with id k. Open PR BUG: Fix OOC Review Findings #1764 keeps this. (related: ENH: Investigate randomized triangle face IDs and metadata alignment #1774) SharedFeatureFaceFilter#1
• MINOR · AddBadDataFilter · AddBadDataFilter.cpp:120 — Both volume fractions are validated even when their noise type is off, so a hidden, unused value fails with -76235/-76236. AddBadDataFilter#6
• MINOR · AlignSectionsFeatureCentroidFilter · AlignSectionsFeatureCentroidFilter.cpp:148 — Reference Slice < 0 is rejected even when Use Reference Slice is off. Also, the slice number counts from the top (slice = Z-1-iter), which is not documented. AlignSectionsFeatureCentroidFilter#5
• MINOR · ApplyTransformationToGeometryFilter · GeometryTransformation.cpp:155 — The interpolation and Cell Attribute Matrix checks run for Translation and Scale too, which never resample, so the default arguments fail with -82006. ApplyTransformationToGeometryFilter#10
• MINOR · ComputeGBPDMetricBasedFilter · ComputeGBPDMetricBasedFilter.cpp:234 — FromSIMPLJson derives the Triangle Geometry from CrystalStructuresArrayPath (the volume data container), so converted pipelines fail parameter validation. Use SurfaceMeshFaceLabelsArrayPath. ComputeGBPDMetricBasedFilter#6
• MINOR · ConditionalSetValueFilter · ConditionalSetValueFilter.cpp:104 — Value To Replace is checked for empty and for type conversion even when Use Conditional Mask hides it. Both empty checks share one error code. ConditionalSetValueFilter#3
• MINOR · FeatureFaceCurvatureFilter · FeatureFaceCurvatureFilter.cpp:71 — The Face Attribute Matrix parameter is never read in preflight or execute; the tuple shape always comes from the geometry's own Face Attribute Matrix. FeatureFaceCurvatureFilter#10
• MINOR · MergeTwinsFilter · MergeTwins.cpp:212 — The generator always uses std::mt19937::default_seed and ignores the Seed Value, so the stored seed is misleading. Only parent labels change, not the merge result. MergeTwinsFilter#1
• MINOR · PadImageGeometryFilter · PadImageGeometryFilter.cpp:129 — The Cell Data parameter is read and stored, but preflight and the algorithm always use the geometry's own Cell Attribute Matrix. PadImageGeometryFilter#6
• MINOR · PartitionGeometryFilter · PartitionGeometryFilter.cpp:507 — DataCheckDimensionality runs in every mode, so a planar Triangle Geometry fails -3040..-3042 in Advanced, Bounding Box and Existing Partition Grid modes too. (related: BUG: Partition Geometry reads unavailable coordinates during preflight #1798) PartitionGeometryFilter#4
• MINOR · ReadImageStackFilter · ReadImageFilter.cpp:306 — NrrdPreflight does not get the read-single-page option: a stack of 3D .nrrd files keeps only the first slice with no -2005 warning, and a Z crop on a stack of 2D .nrrd files fails with -5555. ReadImageStackFilter#8
• MINOR · ReadStringDataArrayFilter · ReadStringDataArrayFilter.cpp:101 — Data Format is read and stored but never passed to CreateStringArrayAction, so the selection has no effect. ReadStringDataArrayFilter#2

Related issues

• BUG: Hierarchical smoothing ignores Max Bisection Iterations #1791 — BUG: Hierarchical smoothing ignores Max Bisection Iterations (same as HierarchicalSmoothFilter#2)
• BUG: Partition Geometry reads unavailable coordinates during preflight #1798 — BUG: Partition Geometry reads unavailable coordinates during preflight (partly overlaps PartitionGeometryFilter#4)
• BUG: Randomize Feature Ids misaligns attributes for longer cycles #1785 — BUG: Randomize Feature Ids misaligns attributes for longer cycles (same as RandomizeFeatureIdsFilter#1)
• ENH: Investigate randomized triangle face IDs and metadata alignment #1774 — ENH: Investigate randomized triangle face IDs and metadata alignment (same as SharedFeatureFaceFilter#1)

Out of scope

• Findings in filters that have a V&V report are excluded from this issue. There is 1 such filter: RemoveFlaggedFeaturesFilter. A separate V&V branch handles it.
• Other defect classes in the same filters. This issue covers only this class.