-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#5145 - HeatExchangerAirToAirSensibleAndLatent normalization divisor error #5193
Conversation
…ding to a zero divisor
bool TableLookup_Impl::setNormalizationDivisor(double normalizationDivisor) { | ||
if (std::abs(normalizationDivisor) < std::numeric_limits<double>::min()) { | ||
LOG(Warn, "Unable to set " << briefDescription() << "'s Normalization divisor to zero."); | ||
return false; | ||
} | ||
|
||
bool result = setDouble(OS_Table_LookupFields::NormalizationDivisor, normalizationDivisor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TableLookup will reject 0.0 now, and return false + a warn
const auto val100 = sensibleEffectivenessat100HeatingAirFlow(); | ||
if (std::abs(val100) < std::numeric_limits<double>::min()) { | ||
LOG_AND_THROW("Unable to set " << briefDescription() | ||
<< "'s Sensible Effectiveness at 75 Heating Air Flow because the value at 100 is zero and would lead to a " | ||
"Normalization divisor equal to zero."); | ||
} | ||
auto tableLookup = makeTable(sensibleEffectivenessat75HeatingAirFlow, val100, model(), fmt::format("{}_SensHeatEff", nameString())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HX: we actually throw if we get to such a condition.
// Ensure we protect users against dumb issues that lead to a normalization divisor of zero | ||
{ | ||
// Ensure no curves assigned for now | ||
hx.resetSensibleEffectivenessofHeatingAirFlowCurve(); | ||
hx.resetLatentEffectivenessofHeatingAirFlowCurve(); | ||
hx.resetSensibleEffectivenessofCoolingAirFlowCurve(); | ||
hx.resetLatentEffectivenessofCoolingAirFlowCurve(); | ||
|
||
// IMHO this doesn't make sense (IDD should be `\minimum> 0.0`), but the E+ IDD and OS IDD have always allowed it, so leaving it untouched | ||
EXPECT_TRUE(hx.setSensibleEffectivenessat100HeatingAirFlow(0.0)); | ||
EXPECT_TRUE(hx.setLatentEffectivenessat100HeatingAirFlow(0.0)); | ||
EXPECT_TRUE(hx.setSensibleEffectivenessat100CoolingAirFlow(0.0)); | ||
EXPECT_TRUE(hx.setLatentEffectivenessat100CoolingAirFlow(0.0)); | ||
|
||
EXPECT_ANY_THROW(hx.setSensibleEffectivenessat75HeatingAirFlow(0.5)); | ||
EXPECT_ANY_THROW(hx.setLatentEffectivenessat75HeatingAirFlow(0.5)); | ||
EXPECT_ANY_THROW(hx.setSensibleEffectivenessat75CoolingAirFlow(0.5)); | ||
EXPECT_ANY_THROW(hx.setLatentEffectivenessat75CoolingAirFlow(0.5)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test that it throws with HX
// #5145 - We reject 0 as a normalization divisor | ||
EXPECT_FALSE(tableLookup.setNormalizationDivisor(0.0)); | ||
EXPECT_EQ(0.5, tableLookup.normalizationDivisor()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test that TableLookup just returns false and leaves it untouched
CI Results for 1b8f688:
|
Pull request overview
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.