Skip to content

Commit 8777659

Browse files
committed
fix(mass_balance): avoid div-by-zero, don't check mass balance with frequency 0
1 parent b8e269f commit 8777659

File tree

2 files changed

+27
-11
lines changed

2 files changed

+27
-11
lines changed

src/utilities/bmi/mass_balance.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,27 @@ NgenMassBalance::NgenMassBalance() : check(false) {}
3737
NgenMassBalance::~NgenMassBalance() = default;
3838

3939
auto NgenMassBalance::run(const ModelPtr& model, const Context& ctx) const -> expected<void, ProtocolError> {
40-
bool check_step = false;
41-
//if frequency was set to -1 (or any negative), only check at the end
42-
if( frequency < 0 ){
43-
if(ctx.current_time_step == ctx.total_steps){
44-
check_step = true;
45-
}
46-
}
47-
else{
48-
check_step = (ctx.current_time_step % frequency) == 0;
49-
}
5040
if( model == nullptr ) {
5141
return make_unexpected<ProtocolError>( ProtocolError(
5242
Error::UNITIALIZED_MODEL,
5343
"Cannot run mass balance protocol with null model."
5444
)
5545
);
46+
} else if( !check || !supported ) {
47+
return {};
5648
}
57-
if(model && supported && check && check_step) {
49+
bool check_step = false;
50+
//if frequency was set to -1 (or any negative), only check at the end
51+
//use <= to avoid a potential divide by zero should frequency be 0
52+
//(though frequency 0 should have been caught during initialization and check disabled)
53+
if( frequency > 0 ){
54+
check_step = (ctx.current_time_step % frequency) == 0;
55+
}
56+
else if(ctx.current_time_step == ctx.total_steps){
57+
check_step = true;
58+
}
59+
60+
if(check_step) {
5861
double mass_in, mass_out, mass_stored, mass_leaked, mass_balance;
5962
model->GetValue(INPUT_MASS_NAME, &mass_in);
6063
model->GetValue(OUTPUT_MASS_NAME, &mass_out);
@@ -174,6 +177,9 @@ auto NgenMassBalance::initialize(const ModelPtr& model, const Properties& proper
174177
} else {
175178
frequency = 1; //default, check every timestep
176179
}
180+
if ( frequency == 0 ) {
181+
check = false; // can't check at frequency 0, disable mass balance checking
182+
}
177183
} else{
178184
//no mass balance requested, or not supported, so don't check it
179185
check = false;

test/utils/bmi/mass_balance_Test.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,16 @@ TEST_F(Bmi_Mass_Balance_Test, frequency) {
369369
ASSERT_THROW(protocols.run(models::bmi::protocols::Protocol::MASS_BALANCE, make_context(2, 2, time, model_name)), ProtocolError);
370370
}
371371

372+
TEST_F(Bmi_Mass_Balance_Test, frequency_zero) {
373+
auto properties = MassBalanceMock(true, 1e-5, 0).as_json_property();
374+
auto context = make_context(0, 2, time, model_name);
375+
auto protocols = NgenBmiProtocols(model, properties);
376+
double mass_error = 10; // Force a mass balance error above tolerance
377+
model->SetValue(OUTPUT_MASS_NAME, &mass_error);
378+
auto result = protocols.run(models::bmi::protocols::Protocol::MASS_BALANCE, make_context(0, 2, time, model_name));
379+
EXPECT_TRUE( result.has_value() ); // should pass, frequency 0 means never check
380+
}
381+
372382
TEST_F(Bmi_Mass_Balance_Test, frequency_end) {
373383
auto properties = MassBalanceMock(true, 1e-5, -1).as_json_property();
374384
auto context = make_context(0, 2, time, model_name);

0 commit comments

Comments
 (0)