From ad8014bb8dfbd26e6777ab1d0561368a7426df54 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 May 2024 16:54:32 +0200 Subject: [PATCH 1/3] Fix typo --- src/model/TableLookup.cpp | 8 ++++---- src/model/TableLookup.hpp | 2 +- src/model/TableLookup_Impl.hpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/model/TableLookup.cpp b/src/model/TableLookup.cpp index b85107ac973..5c399c64d32 100644 --- a/src/model/TableLookup.cpp +++ b/src/model/TableLookup.cpp @@ -165,8 +165,8 @@ namespace model { return result; } - bool TableLookup_Impl::setNormalizationDivisor(double normalizationDivisior) { - bool result = setDouble(OS_Table_LookupFields::NormalizationDivisor, normalizationDivisior); + bool TableLookup_Impl::setNormalizationDivisor(double normalizationDivisor) { + bool result = setDouble(OS_Table_LookupFields::NormalizationDivisor, normalizationDivisor); return result; } @@ -374,8 +374,8 @@ namespace model { return getImpl()->setNormalizationMethod(normalizationMethod); } - bool TableLookup::setNormalizationDivisor(double normalizationDivisior) { - return getImpl()->setNormalizationDivisor(normalizationDivisior); + bool TableLookup::setNormalizationDivisor(double normalizationDivisor) { + return getImpl()->setNormalizationDivisor(normalizationDivisor); } bool TableLookup::setMinimumOutput(double minimumOutput) { diff --git a/src/model/TableLookup.hpp b/src/model/TableLookup.hpp index 2d17a155b31..04ce8ec2a0c 100644 --- a/src/model/TableLookup.hpp +++ b/src/model/TableLookup.hpp @@ -69,7 +69,7 @@ namespace model { bool setNormalizationMethod(const std::string& normalizationMethod); - bool setNormalizationDivisor(double normalizationDivisior); + bool setNormalizationDivisor(double normalizationDivisor); bool setMinimumOutput(double minimumOutput); void resetMinimumOutput(); diff --git a/src/model/TableLookup_Impl.hpp b/src/model/TableLookup_Impl.hpp index 24c4d075a98..43e913f9200 100644 --- a/src/model/TableLookup_Impl.hpp +++ b/src/model/TableLookup_Impl.hpp @@ -72,7 +72,7 @@ namespace model { bool setNormalizationMethod(const std::string& normalizationMethod); - bool setNormalizationDivisor(double normalizationDivisior); + bool setNormalizationDivisor(double normalizationDivisor); bool setMinimumOutput(double minimumOutput); void resetMinimumOutput(); From 04636b49cecd10ec7d0256185e741ed93d1bc98a Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 May 2024 17:13:41 +0200 Subject: [PATCH 2/3] Fix #5145 - Reject 0.0 as a normalization divisor for TableLookup Same as https://github.com/NREL/EnergyPlus/blob/22053e45b7af9fbe952327462d1852378158b128/src/EnergyPlus/CurveManager.cc#L2497-L2501 --- src/model/TableLookup.cpp | 6 ++++++ src/model/test/TableLookup_GTest.cpp | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/model/TableLookup.cpp b/src/model/TableLookup.cpp index 5c399c64d32..0ace1a568d2 100644 --- a/src/model/TableLookup.cpp +++ b/src/model/TableLookup.cpp @@ -26,6 +26,7 @@ #include #include +#include namespace openstudio { namespace model { @@ -166,6 +167,11 @@ namespace model { } bool TableLookup_Impl::setNormalizationDivisor(double normalizationDivisor) { + if (std::abs(normalizationDivisor) < std::numeric_limits::min()) { + LOG(Warn, "Unable to set " << briefDescription() << "'s Normalization divisor to zero."); + return false; + } + bool result = setDouble(OS_Table_LookupFields::NormalizationDivisor, normalizationDivisor); return result; } diff --git a/src/model/test/TableLookup_GTest.cpp b/src/model/test/TableLookup_GTest.cpp index 4156343639d..cb7d7d0152e 100644 --- a/src/model/test/TableLookup_GTest.cpp +++ b/src/model/test/TableLookup_GTest.cpp @@ -51,6 +51,10 @@ TEST_F(ModelFixture, TableLookup_GettersSetters) { EXPECT_TRUE(tableLookup.setNormalizationDivisor(0.5)); EXPECT_EQ(0.5, tableLookup.normalizationDivisor()); + // #5145 - We reject 0 as a normalization divisor + EXPECT_FALSE(tableLookup.setNormalizationDivisor(0.0)); + EXPECT_EQ(0.5, tableLookup.normalizationDivisor()); + // Minimum Output: Optional Double EXPECT_TRUE(tableLookup.setMinimumOutput(0.6)); ASSERT_TRUE(tableLookup.minimumOutput()); From 1b8f688f27891d071ef53403483e73a9b450ad84 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 13 May 2024 17:20:00 +0200 Subject: [PATCH 3/3] Further protect users from mistakes in HX backward compat methods leading to a zero divisor --- ...HeatExchangerAirToAirSensibleAndLatent.cpp | 38 +++++++++++++++---- ...changerAirToAirSensibleAndLatent_GTest.cpp | 20 ++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/model/HeatExchangerAirToAirSensibleAndLatent.cpp b/src/model/HeatExchangerAirToAirSensibleAndLatent.cpp index 55209a07cb8..328ac5a9296 100644 --- a/src/model/HeatExchangerAirToAirSensibleAndLatent.cpp +++ b/src/model/HeatExchangerAirToAirSensibleAndLatent.cpp @@ -35,6 +35,8 @@ // Remove when deprecated methods are removed #include "../utilities/core/DeprecatedHelpers.hpp" +#include + namespace openstudio { namespace model { @@ -1026,8 +1028,13 @@ namespace model { return setCurveAt75(*curve_, sensibleEffectivenessat75HeatingAirFlow); } - auto tableLookup = makeTable(sensibleEffectivenessat75HeatingAirFlow, sensibleEffectivenessat100HeatingAirFlow(), model(), - fmt::format("{}_SensHeatEff", nameString())); + const auto val100 = sensibleEffectivenessat100HeatingAirFlow(); + if (std::abs(val100) < std::numeric_limits::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())); return setSensibleEffectivenessofHeatingAirFlowCurve(tableLookup); } @@ -1038,8 +1045,13 @@ namespace model { return setCurveAt75(*curve_, latentEffectivenessat75HeatingAirFlow); } - auto tableLookup = - makeTable(latentEffectivenessat75HeatingAirFlow, latentEffectivenessat100HeatingAirFlow(), model(), fmt::format("{}_LatHeatEff", nameString())); + const auto val100 = latentEffectivenessat100HeatingAirFlow(); + if (std::abs(val100) < std::numeric_limits::min()) { + LOG_AND_THROW("Unable to set " << briefDescription() + << "'s Latent 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(latentEffectivenessat75HeatingAirFlow, val100, model(), fmt::format("{}_LatHeatEff", nameString())); return setLatentEffectivenessofHeatingAirFlowCurve(tableLookup); } @@ -1050,8 +1062,13 @@ namespace model { return setCurveAt75(*curve_, sensibleEffectivenessat75CoolingAirFlow); } - auto tableLookup = makeTable(sensibleEffectivenessat75CoolingAirFlow, sensibleEffectivenessat100CoolingAirFlow(), model(), - fmt::format("{}_SensCoolEff", nameString())); + const auto val100 = sensibleEffectivenessat100CoolingAirFlow(); + if (std::abs(val100) < std::numeric_limits::min()) { + LOG_AND_THROW("Unable to set " << briefDescription() + << "'s Sensible Effectiveness at 75 Cooling Air Flow because the value at 100 is zero and would lead to a " + "Normalization divisor equal to zero."); + } + auto tableLookup = makeTable(sensibleEffectivenessat75CoolingAirFlow, val100, model(), fmt::format("{}_SensCoolEff", nameString())); return setSensibleEffectivenessofCoolingAirFlowCurve(tableLookup); } @@ -1062,8 +1079,13 @@ namespace model { return setCurveAt75(*curve_, latentEffectivenessat75CoolingAirFlow); } - auto tableLookup = - makeTable(latentEffectivenessat75CoolingAirFlow, latentEffectivenessat100CoolingAirFlow(), model(), fmt::format("{}_LatCoolEff", nameString())); + const auto val100 = latentEffectivenessat100CoolingAirFlow(); + if (std::abs(val100) < std::numeric_limits::min()) { + LOG_AND_THROW("Unable to set " << briefDescription() + << "'s Latent Effectiveness at 75 Cooling Air Flow because the value at 100 is zero and would lead to a " + "Normalization divisor equal to zero."); + } + auto tableLookup = makeTable(latentEffectivenessat75CoolingAirFlow, val100, model(), fmt::format("{}_LatCoolEff", nameString())); return setLatentEffectivenessofCoolingAirFlowCurve(tableLookup); } diff --git a/src/model/test/HeatExchangerAirToAirSensibleAndLatent_GTest.cpp b/src/model/test/HeatExchangerAirToAirSensibleAndLatent_GTest.cpp index be3ccabcfe2..9aea91c6a64 100644 --- a/src/model/test/HeatExchangerAirToAirSensibleAndLatent_GTest.cpp +++ b/src/model/test/HeatExchangerAirToAirSensibleAndLatent_GTest.cpp @@ -401,6 +401,26 @@ TEST_F(ModelFixture, HeatExchangerAirToAirSensibleAndLatent_Deprecated75Effectiv EXPECT_TRUE(hx.setLatentEffectivenessat75CoolingAirFlow(val2)); EXPECT_DOUBLE_EQ(val2, hx.latentEffectivenessat75CoolingAirFlow()); } + + // 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)); + } } TEST_F(ModelFixture, HeatExchangerAirToAirSensibleAndLatent_assignHistoricalEffectivenessCurves) {