Skip to content

Commit 3c70392

Browse files
imikejacksonclaude
andcommitted
fix(filter): drop dead seed write, reorder VF checks, tidy per review
- Remove params.seed = m_InputValues->seed (dead write; rng is seeded directly) - Add clarifying comment noting simulateMTR uses the rng, not SimulationParams::seed - Remove unused #include <cmath> from MTRSim.cpp - Move per-element VF range check (-13007) before sum check (-13003) so out-of-range values produce the precise diagnostic rather than the misleading sum message - Remove redundant size/spacing/VF args.insertOrAssign calls in execute test that duplicated what MakeValidArgs already sets; keep seed overrides Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b107269 commit 3c70392

3 files changed

Lines changed: 9 additions & 13 deletions

File tree

src/MTRSim/Filters/Algorithms/MTRSim.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include <fmt/format.h>
99

10-
#include <cmath>
1110
#include <exception>
1211
#include <random>
1312
#include <vector>
@@ -62,7 +61,7 @@ Result<> MTRSim::operator()()
6261
params.dz = m_InputValues->physicalSpacing[2];
6362
params.volumeFractions = m_InputValues->volumeFractions[0]; // 1 row
6463
params.thetaList = m_InputValues->thetaList;
65-
params.seed = m_InputValues->seed;
64+
// Note: simulateMTR is driven by the seeded rng passed below; SimulationParams::seed is not consulted.
6665

6766
if(m_ShouldCancel)
6867
{

src/MTRSim/Filters/MTRSimFilter.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ IFilter::PreflightResult MTRSimFilter::preflightImpl(const DataStructure& dataSt
152152
{
153153
return {MakeErrorResult<OutputActions>(-13002, fmt::format("Volume Fraction must be 1 row x {} columns (one per ODF component).", numComponents))};
154154
}
155+
for(double v : pVolumeFractions[0])
156+
{
157+
if(v < 0.0 || v > 1.0)
158+
{
159+
return {MakeErrorResult<OutputActions>(-13007, fmt::format("Each Volume Fraction value must be in the range [0, 1] (got {:.4f}).", v))};
160+
}
161+
}
155162
double vfSum = 0.0;
156163
for(double v : pVolumeFractions[0])
157164
{
@@ -161,13 +168,6 @@ IFilter::PreflightResult MTRSimFilter::preflightImpl(const DataStructure& dataSt
161168
{
162169
return {MakeErrorResult<OutputActions>(-13003, fmt::format("Volume Fraction values must sum to 1.0 (got {:.4f}).", vfSum))};
163170
}
164-
for(double v : pVolumeFractions[0])
165-
{
166-
if(v < 0.0 || v > 1.0)
167-
{
168-
return {MakeErrorResult<OutputActions>(-13007, fmt::format("Each Volume Fraction value must be in the range [0, 1] (got {:.4f}).", v))};
169-
}
170-
}
171171
if(pThetaList.size() < numComponents - 1)
172172
{
173173
return {MakeErrorResult<OutputActions>(-13004, fmt::format("Theta List needs at least {} rows (components - 1).", numComponents - 1))};

test/MTRSimTest.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,7 @@ TEST_CASE("MTRSim::MTRSimFilter: Execute wires simulation to output arrays", "[M
133133

134134
MTRSimFilter filter;
135135
Arguments args = MakeValidArgs(compPaths);
136-
// Modest domain: 100x100 = 10000 voxels (Z=0 -> single layer).
137-
args.insertOrAssign(MTRSimFilter::k_PhysicalSize_Key, std::vector<float32>{2.0f, 2.0f, 0.0f});
138-
args.insertOrAssign(MTRSimFilter::k_PhysicalSpacing_Key, std::vector<float32>{0.02f, 0.02f, 0.02f});
139-
args.insertOrAssign(MTRSimFilter::k_VolumeFractions_Key, DynamicTableParameter::ValueType{{0.30, 0.35, 0.35}});
136+
// size/spacing/VF come from MakeValidArgs (100x100 grid)
140137
args.insertOrAssign(MTRSimFilter::k_UseSeed_Key, true);
141138
args.insertOrAssign(MTRSimFilter::k_SeedValue_Key, static_cast<uint64>(42));
142139

0 commit comments

Comments
 (0)