Skip to content

Commit 38bb3e0

Browse files
imikejacksonclaude
andcommitted
fix(filter): harden MTRSimFilter preflight + conventions per review
- Add zero-spacing guard (-13006) before dim() divisions in preflightImpl - Update Physical Size help text to document the 2D (Z=0) mode - Change physicalSize/physicalSpacing members from std::vector<float> to std::vector<float32> to match what executeImpl reads from filterArgs - Change outputs separator label to "Output Data Object(s)" (matches ReadMTRSimODFFilter) - Remove premature #include "simplnx/DataStructure/DataArray.hpp" from stub MTRSim.cpp - Add preflight test for Theta List rows with wrong column count (-13005) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 14fbb90 commit 38bb3e0

4 files changed

Lines changed: 28 additions & 6 deletions

File tree

src/MTRSim/Filters/Algorithms/MTRSim.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#include "MTRSim.hpp"
22

3-
#include "simplnx/DataStructure/DataArray.hpp"
4-
53
using namespace nx::core;
64

75
// -----------------------------------------------------------------------------

src/MTRSim/Filters/Algorithms/MTRSim.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ struct MTRSIM_EXPORT MTRSimInputValues
1717
std::vector<DataPath> odfComponentPaths;
1818
std::vector<std::vector<double>> volumeFractions; // 1 row x N cols
1919
std::vector<std::vector<double>> thetaList; // M rows x 3 cols
20-
std::vector<float> physicalSize; // [x,y,z] microns
21-
std::vector<float> physicalSpacing; // [x,y,z] microns
20+
std::vector<float32> physicalSize; // [x,y,z] microns
21+
std::vector<float32> physicalSpacing; // [x,y,z] microns
2222
uint64 seed;
2323
bool generatePolarColoring;
2424
DataPath outputGeometryPath;

src/MTRSim/Filters/MTRSimFilter.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ Parameters MTRSimFilter::parameters() const
8686
"Size/Spacing.",
8787
thetaInfo));
8888
}
89-
params.insert(std::make_unique<VectorFloat32Parameter>(k_PhysicalSize_Key, "Physical Size (microns)", "Domain extent X,Y,Z.", std::vector<float32>{38.1f, 12.7f, 0.0f},
89+
params.insert(std::make_unique<VectorFloat32Parameter>(k_PhysicalSize_Key, "Physical Size (microns)", "Domain extent X, Y, Z in microns. Set Z = 0 to generate a single-layer (2D) microstructure.", std::vector<float32>{38.1f, 12.7f, 0.0f},
9090
std::vector<std::string>{"X", "Y", "Z"}));
9191
params.insert(std::make_unique<VectorFloat32Parameter>(k_PhysicalSpacing_Key, "Physical Spacing (microns)", "Voxel spacing X,Y,Z.", std::vector<float32>{0.02f, 0.02f, 0.02f},
9292
std::vector<std::string>{"X", "Y", "Z"}));
@@ -96,7 +96,7 @@ Parameters MTRSimFilter::parameters() const
9696
params.insert(std::make_unique<NumberParameter<uint64>>(k_SeedValue_Key, "Seed Value", "The seed fed into the random generator.", std::mt19937::default_seed));
9797
params.insert(std::make_unique<DataObjectNameParameter>(k_SeedArrayName_Key, "Stored Seed Value Array Name", "Top-level array recording the seed used.", "MTRSim SeedValue"));
9898

99-
params.insertSeparator(Parameters::Separator{"Outputs"});
99+
params.insertSeparator(Parameters::Separator{"Output Data Object(s)"});
100100
params.insertLinkableParameter(
101101
std::make_unique<BoolParameter>(k_GeneratePolarColoring_Key, "Generate Polar Coloring", "Create a 3-component UInt8 RGB array using the MATLAB polar color mapping.", false));
102102
params.insert(std::make_unique<DataGroupCreationParameter>(k_OutputGeometry_Key, "Output Image Geometry", "Path of the new microstructure Image Geometry.", DataPath({"MTR Microstructure"})));
@@ -173,6 +173,11 @@ IFilter::PreflightResult MTRSimFilter::preflightImpl(const DataStructure& dataSt
173173
}
174174
}
175175

176+
if(pSpacing[0] <= 0.0f || pSpacing[1] <= 0.0f)
177+
{
178+
return {MakeErrorResult<OutputActions>(-13006, "Physical Spacing X and Y must be greater than 0.")};
179+
}
180+
176181
const auto dim = [](float len, float sp) { return static_cast<usize>(std::max(std::lround(len / sp), 1L)); };
177182
const usize nx = dim(pSize[0], pSpacing[0]);
178183
const usize ny = dim(pSize[1], pSpacing[1]);

test/MTRSimTest.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* 2. Theta List rows < numComponents - 1 -> invalid.
88
* 3. Volume Fraction values do not sum to 1.0 -> invalid.
99
* 4. Happy-path args -> preflight VALID (builds geometry + array actions).
10+
* 5. Theta List rows with wrong column count (2 instead of 3) -> invalid (-13005).
1011
*/
1112

1213
#include <catch2/catch.hpp>
@@ -146,3 +147,21 @@ TEST_CASE("MTRSim::MTRSimFilter: Rejects Volume Fraction not summing to 1.0", "[
146147
auto preflightResult = filter.preflight(dataStructure, args);
147148
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
148149
}
150+
151+
TEST_CASE("MTRSim::MTRSimFilter: Rejects Theta List rows with wrong column count", "[MTRSim][MTRSimFilter][ErrorPath]")
152+
{
153+
UnitTest::LoadPlugins();
154+
155+
DataStructure dataStructure;
156+
const std::vector<DataPath> compPaths = BuildOdfDataStructure(dataStructure, 3);
157+
158+
MTRSimFilter filter;
159+
Arguments args = MakeValidArgs(compPaths);
160+
// 2 rows supplied (enough for 3 components: needs >= 2), but each row has only 2
161+
// columns instead of 3 — must trigger the column-count check (-13005), not the
162+
// row-count check (-13004).
163+
args.insertOrAssign(MTRSimFilter::k_ThetaList_Key, DynamicTableParameter::ValueType{{0.1, 0.45}, {0.08, 0.37}});
164+
165+
auto preflightResult = filter.preflight(dataStructure, args);
166+
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
167+
}

0 commit comments

Comments
 (0)