-
Notifications
You must be signed in to change notification settings - Fork 192
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
v23.2.0-IOFreeze: Coil and Unitary systems changes #4958
Conversation
…(Two Speed and VSD)
… on Unitary or ZoneHVAC:WaterToAirHeatPump * OS:Coil:Cooling:WaterToAirHeatPump:EquationFit * OS:Coil:Cooling:WaterToAirHeatPump:VariableSpeedEquationFit * OS:Coil:Heating:WaterToAirHeatPump:EquationFit * OS:Coil:Cooling:DX:VariableSpeed
…m and ZoneHVACWaterToAirHeatPump
…onal) for all remaining 10 coils * OS:Coil:Cooling:DX:CurveFit:Performance * OS:Coil:Cooling:DX:SingleSpeed * OS:Coil:Cooling:DX:TwoStageWithHumidityControlMode * OS:Coil:Cooling:DX:MultiSpeed * OS:Coil:Heating:DX:SingleSpeed * OS:Coil:Heating:DX:MultiSpeed * OS:Coil:Heating:DX:VariableSpeed * OS:Coil:WaterHeating:AirToWaterHeatPump (:Pumped) * OS:Coil:WaterHeating:AirToWaterHeatPump:VariableSpeed * OS:Coil:WaterHeating:AirToWaterHeatPump:Wrapped
… so pragma ignore
fix ruby test setup issues as well
double AirLoopHVACUnitarySystem::maximumCyclingRate() const { | ||
DEPRECATED_AT_MSG(3, 7, 0, | ||
"As of EnergyPlus 23.2.0, this property is on the child cooling coil. Use coolingCoil()->maximumCyclingRate() instead."); | ||
double result = 2.5; // former default | ||
if (auto c_ = coolingCoil()) { | ||
if (auto c_eq_ = c_->optionalCast<CoilCoolingWaterToAirHeatPumpEquationFit>()) { | ||
result = c_eq_->maximumCyclingRate(); | ||
} else if (auto c_vs_ = c_->optionalCast<CoilCoolingWaterToAirHeatPumpVariableSpeedEquationFit>()) { | ||
result = c_vs_->maximumCyclingRate(); | ||
} else if (auto c_dx_ = c_->optionalCast<CoilCoolingDXVariableSpeed>()) { | ||
result = c_dx_->maximumCyclingRate(); | ||
} | ||
} | ||
return result; | ||
} |
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.
Deprecated the former model methods for the unitary systems, trying to set it on the child cooling coil.
Note: This is does not modify the "Part Load Fraction Correlation Curve" on the heating coil (which is created during VT using some calculations based on Max Cycling Rate and Heat Pump Time Constant). That would be very weird IMHO.
I actually considering just making the deprecated method be a no-op and return false
double ratedHighSpeedEvaporatorFanPowerPerVolumeFlowRate2017() const; | ||
|
||
double ratedHighSpeedEvaporatorFanPowerPerVolumeFlowRate2023() const; |
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.
Example for the new Fan power per vol flow rate 2017 / 2023. Added to model here.
idfObject.setDouble(Coil_Cooling_DX_TwoSpeedFields::HighSpeedRatedEvaporatorFanPowerPerVolumeFlowRate2017, | ||
modelObject.ratedHighSpeedEvaporatorFanPowerPerVolumeFlowRate2017()); | ||
idfObject.setDouble(Coil_Cooling_DX_TwoSpeedFields::HighSpeedRatedEvaporatorFanPowerPerVolumeFlowRate2023, | ||
modelObject.ratedHighSpeedEvaporatorFanPowerPerVolumeFlowRate2023()); |
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.
FT'ed here.
@@ -86,6 +86,8 @@ namespace model { | |||
|
|||
double crankcaseHeaterCapacity() const; | |||
|
|||
boost::optional<Curve> crankcaseHeaterCapacityFunctionofTemperatureCurve() const; |
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.
cranckcase curve example. This curve is optional everywhere.
bool setCrankcaseHeaterCapacityFunctionofTemperatureCurve(const Curve& curve); | ||
void resetCrankcaseHeaterCapacityFunctionofTemperatureCurve(); |
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.
has a set and a reset accordingly
} else if (iddname == "OS:AirLoopHVAC:UnitarySystem") { | ||
|
||
// Removed fields | ||
// 38 - Maximum Cycling Rate | ||
// 39 - Heat Pump Time Constant | ||
// 40 - Fraction of On-Cycle Power Use | ||
// 41 - Heat Pump Fan Delay Time | ||
|
||
auto iddObject = idd_3_7_0.getObject(iddname); | ||
IdfObject newObject(iddObject.get()); | ||
|
||
for (size_t i = 0; i < object.numFields(); ++i) { | ||
if ((value = object.getString(i))) { | ||
if (i < 38) { | ||
newObject.setString(i, value.get()); | ||
} else if (i > 41) { | ||
newObject.setString(i - 4, value.get()); | ||
} | ||
} | ||
} | ||
|
||
m_refactored.push_back(RefactoredObjectData(object, newObject)); | ||
ss << newObject; | ||
|
||
auto it = CoilLatentTransitionInfo::findFromParent(coilTransitionInfos, object); | ||
if (it != coilTransitionInfos.end()) { | ||
if (it->isCurveCreationNeeded()) { | ||
IdfObject plfCurve = it->createCurveLinear(idd_3_7_0); | ||
ss << plfCurve; | ||
m_new.emplace_back(std::move(plfCurve)); | ||
} | ||
} |
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.
When you get to a Unitary System / WAHP, you remove the fields.
You check if a Curve needs to be created, and only if so, you do it.
This ensures that if you have a Unitary System with a Coil:Heating:WaterToAirHeatPump:EquationFit AND a Coil:Cooling:WaterToAirHeatPump:EquationFit, you do create only one unique curve, for both objects to reference.
} else if (iddname == "OS:Coil:Cooling:WaterToAirHeatPump:EquationFit") { | ||
|
||
// Fields that have been added from 3.6.1 to 3.7.0: | ||
// ------------------------------------------------ | ||
// * Part Load Fraction Correlation Curve Name* 17 | ||
// New at end | ||
// * Maximum Cycling Rate * 20 | ||
// * Latent Capacity Time Constant * 21 | ||
// * Fan Delay Time * 22 | ||
|
||
auto iddObject = idd_3_7_0.getObject(iddname); | ||
IdfObject newObject(iddObject.get()); | ||
|
||
for (size_t i = 0; i < object.numFields(); ++i) { | ||
if ((value = object.getString(i))) { | ||
if (i < 17) { | ||
newObject.setString(i, value.get()); | ||
} else { | ||
newObject.setString(i + 1, value.get()); | ||
} | ||
} | ||
} | ||
|
||
auto it = CoilLatentTransitionInfo::findFromCoolingCoil(coilTransitionInfos, object); | ||
const bool hasCoilInfo = (it != coilTransitionInfos.end()); | ||
const std::string curveName = hasCoilInfo ? it->curveName() : fmt::format("{}-PLFCorrelationCurve", object.nameString()); | ||
newObject.setString(17, curveName); | ||
|
||
double maxCyclingRate = 0.0; | ||
double heatPumpTimeConst = 0.0; | ||
double hpDelayTime = 60.0; | ||
if (hasCoilInfo) { | ||
maxCyclingRate = it->maxCyclingRate; | ||
heatPumpTimeConst = it->heatPumpTimeConst; | ||
hpDelayTime = it->hpDelayTime; | ||
} | ||
|
||
newObject.setDouble(20, maxCyclingRate); | ||
newObject.setDouble(21, heatPumpTimeConst); | ||
newObject.setDouble(22, hpDelayTime); | ||
|
||
m_refactored.push_back(RefactoredObjectData(object, newObject)); | ||
ss << newObject; | ||
|
||
if (!hasCoilInfo) { | ||
IdfObject plfCurve = CoilLatentTransitionInfo::defaultHeatPumpCoilPLFCorrelationCurve(idd_3_7_0, curveName); | ||
ss << plfCurve; | ||
m_new.emplace_back(std::move(plfCurve)); | ||
} | ||
|
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.
When you get to a coil that is supposed to inherit from a potential unitary parent
If you have a unitary parent:
- You use the values from the unitary, and the PLF curve name we generated automatically
You don't have a unitary parent:
- You use the defaults from the IDD (moved to Ctor in model since I made it required-field)
- You create a PLF curve calculated from these defaults
@@ -2548,3 +2553,892 @@ TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_GroundHeatExchangerVertical) { | |||
EXPECT_EQ(3.2, uka.getDouble(6).get()); // Average Amplitude of Surface Temperature | |||
EXPECT_EQ(8.0, uka.getDouble(7).get()); // Phase Shift of Minimum Surface Temperature | |||
} | |||
|
|||
TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_Coils_RatedFanPowerPerVolumeFlowRate) { |
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.
LOTS of testing here
Easy, the 2017 / 2023 fan power per vol flow rate.
} | ||
} | ||
|
||
TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_Coils_CrankcaseCurve) { |
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 all crankcase curve insertions
} | ||
} | ||
|
||
TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_Coils_Latent_solo) { |
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.
Starting here you have 600 lines of testing for the Coil Latent Stuff, testing all combinations of Unitary/Child coils I could think off. Making sure the PLF Curves are created properly, shared between HC and CC if appropriate, etc.
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.
This one is certainly a beast. Looks like all the pieces are here: all the FT updates, model and test updates, VT and test updates. I'm surprised there wouldn't be more FT test updates? Also, do we want to handle the "Design Specification Multispeed Object Type" and "Design Specification Multispeed Object Name" additions here - or does it make sense to keep that separated?
@@ -30845,7 +30849,7 @@ Coil:Cooling:DX:TwoSpeed, | |||
\minimum 0.0 | |||
\maximum 1250.0 | |||
\default 773.3 | |||
N13, \field Low Speed 2023 Rated Evaporator Fan Power Per Volume Flow Rate | |||
N13, \field Low Speed Rated Evaporator Fan Power Per Volume Flow Rate 2023 |
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.
This is the approach we've taken across all object/fields, right?
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.
Yeah the reason is dumb, but this is because of our parser...
"Speed 1 field 2023" -> 1 is removed, 2023 is kept
"Speed 2023 field" -> 2023 is removed
@@ -124,6 +124,14 @@ namespace energyplus { | |||
} | |||
} | |||
|
|||
// Part Load Fraction Correlation Curve Name |
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.
Should the PR description be updated for handling new part load fraction correlation curve fields? (similar to addition of crankcase)
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.
I clarified it in the OP.
Coil Latent Stuff:
- * Move some values from unitary systems to child object
+ * Move some values from unitary systems to child object: maximum Cycling Rate, Heat Pump Time Constant, Fan Delay Time => get moved as fields for cooling coil or as the "Part Load Fraction Correlation Curve" in case of heating
I think I mostly got tired towards the end. Lots of these objects don't have good testing (or no testing) to begin with too. You're right to mention it though, it's a good idea to add the tests... I'll muster the courage to write them.
This is a gigantic PR already, so I figured we should do it separately. |
@@ -31279,7 +31279,7 @@ Coil:Cooling:DX:VariableSpeed, | |||
\memo wet coil when compressor cycles off with continuous fan operation. Requires two to | |||
\memo ten sets of performance data and will interpolate between speeds. Modeled as a | |||
\memo single coil with variable-speed compressor. | |||
\extensible:10 | |||
\extensible:12 |
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.
Fix extensible
@@ -32901,7 +32901,7 @@ Coil:Heating:DX:VariableSpeed, | |||
\memo (includes electric compressor and outdoor fan), variable-speed, with defrost | |||
\memo controls. Requires two to ten sets of performance data and will interpolate between | |||
\memo speeds. | |||
\extensible:7 | |||
\extensible:9 |
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.
Fix extensible
@@ -376,7 +376,7 @@ namespace model { | |||
const CoilCoolingWaterToAirHeatPumpVariableSpeedEquationFitSpeedData& speed) { | |||
auto modelObjectList = speedDataList(); | |||
if (modelObjectList) { | |||
modelObjectList->addModelObject(speed); | |||
return modelObjectList->addModelObject(speed); |
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.
Some addSpeed methods weren't returning true on success
Model m; | ||
CoilHeatingDXVariableSpeed coil(m); | ||
|
||
// Indoor Air Inlet Node Name: Required Object | ||
// Indoor Air Outlet Node Name: Required Object | ||
|
||
// Nominal Speed Level: Required Integer | ||
EXPECT_EQ(1, coil.nominalSpeedLevel()); | ||
EXPECT_TRUE(coil.setNominalSpeedLevel(5)); | ||
EXPECT_EQ(5, coil.nominalSpeedLevel()); | ||
|
||
// Rated Heating Capacity At Selected Nominal Speed Level: Required Double | ||
EXPECT_TRUE(coil.isRatedHeatingCapacityAtSelectedNominalSpeedLevelAutosized()); | ||
// Set | ||
EXPECT_TRUE(coil.setRatedHeatingCapacityAtSelectedNominalSpeedLevel(1000.6)); | ||
ASSERT_TRUE(coil.ratedHeatingCapacityAtSelectedNominalSpeedLevel()); | ||
EXPECT_EQ(1000.6, coil.ratedHeatingCapacityAtSelectedNominalSpeedLevel().get()); | ||
EXPECT_FALSE(coil.isRatedHeatingCapacityAtSelectedNominalSpeedLevelAutosized()); | ||
// Autosize | ||
coil.autosizeRatedHeatingCapacityAtSelectedNominalSpeedLevel(); | ||
EXPECT_TRUE(coil.isRatedHeatingCapacityAtSelectedNominalSpeedLevelAutosized()); |
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.
Add model tests for some that were really untested
Test/CoilCoolingWaterToAirHeatPumpEquationFit_GTest.cpp | ||
Test/CoilCoolingWaterToAirHeatPumpVariableSpeedEquationFit_GTest.cpp | ||
Test/CoilHeatingDXMultiSpeed_GTest.cpp | ||
Test/CoilHeatingDXSingleSpeed_GTest.cpp | ||
Test/CoilHeatingDXVariableSpeed_GTest.cpp | ||
Test/CoilHeatingElectricMultiStage_GTest.cpp | ||
Test/CoilHeatingGas_GTest.cpp | ||
Test/CoilHeatingGasMultiStage_GTest.cpp | ||
Test/CoilHeatingWaterToAirHeatPumpEquationFit_GTest.cpp | ||
Test/CoilHeatingWaterToAirHeatPumpVariableSpeedEquationFit_GTest.cpp | ||
Test/CoilSystemIntegratedHeatPumpAirSource_GTest.cpp | ||
Test/CoilUserDefined_GTest.cpp | ||
Test/CoilWaterHeatingAirToWaterHeatPump_GTest.cpp | ||
Test/CoilWaterHeatingAirToWaterHeatPumpWrapped_GTest.cpp |
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.
Lots of brand new tests for objects that had no FT tests. Testing all fields
ScheduleConstant availSch(m); | ||
availSch.setName("Coil Avail Sch"); | ||
CurveBiquadratic totalCoolingCapacityFunctionOfTemperatureCurve(m); | ||
totalCoolingCapacityFunctionOfTemperatureCurve.setName("totalCoolingCapacityFunctionOfTemperatureCurve"); | ||
CurveQuadratic totalCoolingCapacityFunctionOfFlowFractionCurve(m); | ||
totalCoolingCapacityFunctionOfFlowFractionCurve.setName("totalCoolingCapacityFunctionOfFlowFractionCurve"); | ||
CurveBiquadratic energyInputRatioFunctionOfTemperatureCurve(m); | ||
energyInputRatioFunctionOfTemperatureCurve.setName("energyInputRatioFunctionOfTemperatureCurve"); | ||
CurveQuadratic energyInputRatioFunctionOfFlowFractionCurve(m); | ||
energyInputRatioFunctionOfFlowFractionCurve.setName("energyInputRatioFunctionOfFlowFractionCurve"); | ||
CurveQuadratic partLoadFractionCorrelationCurve(m); | ||
partLoadFractionCorrelationCurve.setName("partLoadFractionCorrelationCurve"); | ||
|
||
CoilCoolingDXSingleSpeed coil(m, availSch, totalCoolingCapacityFunctionOfTemperatureCurve, totalCoolingCapacityFunctionOfFlowFractionCurve, | ||
energyInputRatioFunctionOfTemperatureCurve, energyInputRatioFunctionOfFlowFractionCurve, | ||
partLoadFractionCorrelationCurve); | ||
|
||
AirLoopHVAC a(m); | ||
auto supplyOutletNode = a.supplyOutletNode(); | ||
EXPECT_TRUE(coil.addToNode(supplyOutletNode)); | ||
|
||
a.supplyInletNode().setName("Supply Inlet Node"); | ||
a.supplyOutletNode().setName("Supply Outlet Node"); | ||
|
||
EXPECT_TRUE(coil.setName("Coil Cooling DX Single Speed")); | ||
EXPECT_EQ(availSch, coil.availabilitySchedule()); | ||
EXPECT_TRUE(coil.setRatedTotalCoolingCapacity(2000.0)); | ||
EXPECT_TRUE(coil.setRatedSensibleHeatRatio(0.8)); | ||
EXPECT_TRUE(coil.setRatedCOP(4.0)); | ||
EXPECT_TRUE(coil.setRatedAirFlowRate(0.9)); | ||
EXPECT_TRUE(coil.setRatedEvaporatorFanPowerPerVolumeFlowRate2017(817.0)); | ||
EXPECT_TRUE(coil.setRatedEvaporatorFanPowerPerVolumeFlowRate2023(823.0)); | ||
EXPECT_EQ(totalCoolingCapacityFunctionOfTemperatureCurve, coil.totalCoolingCapacityFunctionOfTemperatureCurve()); | ||
EXPECT_EQ(totalCoolingCapacityFunctionOfFlowFractionCurve, coil.totalCoolingCapacityFunctionOfFlowFractionCurve()); | ||
EXPECT_EQ(energyInputRatioFunctionOfTemperatureCurve, coil.energyInputRatioFunctionOfTemperatureCurve()); | ||
EXPECT_EQ(energyInputRatioFunctionOfFlowFractionCurve, coil.energyInputRatioFunctionOfFlowFractionCurve()); | ||
EXPECT_EQ(partLoadFractionCorrelationCurve, coil.partLoadFractionCorrelationCurve()); | ||
coil.setMinimumOutdoorDryBulbTemperatureforCompressorOperation(-7.5); | ||
EXPECT_TRUE(coil.setNominalTimeForCondensateRemovalToBegin(6.0)); | ||
EXPECT_TRUE(coil.setRatioOfInitialMoistureEvaporationRateAndSteadyStateLatentCapacity(0.2)); | ||
EXPECT_TRUE(coil.setMaximumCyclingRate(3.2)); | ||
EXPECT_TRUE(coil.setLatentCapacityTimeConstant(62.0)); | ||
EXPECT_TRUE(coil.setCondenserType("EvaporativelyCooled")); | ||
EXPECT_TRUE(coil.setEvaporativeCondenserEffectiveness(0.85)); | ||
EXPECT_TRUE(coil.setEvaporativeCondenserAirFlowRate(1.2)); | ||
EXPECT_TRUE(coil.setEvaporativeCondenserPumpRatedPowerConsumption(51.0)); | ||
EXPECT_TRUE(coil.setCrankcaseHeaterCapacity(105.0)); | ||
CurveLinear crankCurve(m); | ||
crankCurve.setName("CrankHeatCapFT"); | ||
EXPECT_TRUE(coil.setCrankcaseHeaterCapacityFunctionofTemperatureCurve(crankCurve)); | ||
|
||
EXPECT_TRUE(coil.setMaximumOutdoorDryBulbTemperatureForCrankcaseHeaterOperation(9.0)); | ||
EXPECT_TRUE(coil.setBasinHeaterCapacity(90.0)); | ||
EXPECT_TRUE(coil.setBasinHeaterSetpointTemperature(5.0)); | ||
ScheduleConstant basinSch(m); | ||
basinSch.setName("Coil Basin Sch"); | ||
coil.setBasinHeaterOperatingSchedule(basinSch); |
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.
Existing test that had very low coverage: test all fields
…d something more robust, but not bothering now
CI Results for 53a7202:
|
Pull request overview
This is the main dish of the upgrade to v23.2.0 IOFreeze.
Add the new
XXX Fan Power Per Volume Flow Rate 2017/2023
for coilsCoil Latent stuff:
AirLoopHVAC:UnitarySystem
ZoneHVAC:WaterToAirHeatPump
Coil:Cooling:WaterToAirHeatPump:EquationFit
,Coil:Heating:WaterToAirHeatPump:EquationFit
Coil:Cooling:WaterToAirHeatPump:VariableSpeedEquationFit
, (Coil:Heating:WaterToAirHeatPump:VariableSpeedEquationFit
is unchanged actually)Coil:Cooling:DX:VariableSpeed
Add the "Crankcase Heater Capacity Function of Temperature Curve Name" for all coils, see
OpenStudio/src/osversion/VersionTranslator.cpp
Lines 7887 to 7898 in 879a518
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.