ZoneHVAC:*: new Design Specification Multispeed Object fields #4964

joseph-robertson · 2023-09-11T20:36:21Z

Pull request overview

Support new fields "Design Specification Multispeed Object Type" and "Design Specification Multispeed Object Name" for:

ZoneHVAC:WaterToAirHeatPump
ZoneHVAC:TerminalUnit:VariableRefrigerantFlow

Also, add:

UnitarySystemPerformanceMultispeed FT support for CoilXXXDXVariableSpeed and CoilXXXWaterToAirHeatPumpVariableSpeedEquationFit (FT for AirLoopHVACUnitarySystem supports these coil types but FT for UnitarySystemPerformanceMultispeed didn't previously)
a new UnitarySystemPerformanceMultispeed_GTest.cpp FT test file for translating UnitarySystemPerformanceMultispeed on AirLoopHVACUnitarySystem, ZoneHVACWaterToAirHeatPump, and ZoneHVACTerminalUnitVariableRefrigerantFlow

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

resources/model/OpenStudio.idd

jmarrec · 2023-09-12T13:31:45Z

src/model/ZoneHVACWaterToAirHeatPump.cpp

+      if (auto designSpec = designSpecificationMultispeedObject()) {
+        wahpClone.setDesignSpecificationMultispeedObject(designSpec->clone(model).cast<UnitarySystemPerformanceMultispeed>());
+      }


Ok, AirLoopHVACUNitarySystem already clones / remove, so let's do the same

src/model/ZoneHVACWaterToAirHeatPump.cpp

jmarrec · 2023-09-12T13:33:56Z

...energyplus/ForwardTranslator/ForwardTranslateZoneHVACTerminalUnitVariableRefrigerantFlow.cpp

@@ -432,6 +434,18 @@ namespace energyplus {
      }
    }

+    // Design Specification Multispeed Object Name


I don't see any FT testing for this one?

@jmarrec I didn't get too far on this PR yet. I sort of got tripped up on FT. Appears that for AirLoopHVACUnitarySystem, it FTs the UnitarySystemPerformanceMultispeed object, but also creates one if one doesn't exist. Under what conditions should we create one for ZoneHVACWaterToAirHeatPump and ZoneHVACTerminalUnitTVariableRefrigerantFlow?

AirLoopHVACUnitarySystem I believe throws if you use multispeed / variable speed coils without a UnitarySystemPerformanceMultispeed object, hence why the FT creates one if needed.

I'm not 100% sure whether the VRF TU and the ZoneHVAC:WaterToAirHeatPump have the same behavior now. If so, we should create one in FT. In that case, should move the default creation to a helper function so we can reuse in other modules.

oh, that's quite confusing, but it seems that this applies not to the coils but to the fans. The original NFP proposed a new object "FanPerformance:Multispeed", but apparently UnistarySystemPerformance:Multispeed was kept.

https://github.com/NREL/EnergyPlus/blob/develop/design/FY2023/NFP_MultispeedFans.md

My understanding is that this is purely optional, regardless of the coils you use for the ZoneHVAC:WAHP or ZoneHVAC:TU:VRF objects. We should probably make some dummy IDF tests to confirm. I'll ask the E+ dev team right now too.

@jmarrec I stubbed UnitarySystemPerformance FT to handle additional coil types CoilXXXWaterToAirHeatPumpEquationFit, CoilXXXDXVariableRefrigerantFlow, and CoilXXXDXVariableRefrigerantFlowFluidTemperatureControl. But I'm a little unclear on how to determine NumberofSpeedsforXXX. The speeds should not correspond to the coils, but to the supply fan? Are we going to need to create a new setter method for speeds on the UnitarySystemPerformance object?

My earlier post was misleading I think. The object varies the fan airflow in conjunction with the heating/cooling capacity.
So Number of Speeds for Heating should match the heating coil number of speeds, when the coil does have speeds.

I don't think we need to create a UnitarySystemPerformance object for VRF / ZoneHVACWaterToAirHeatPump if they don't throw when one isn't present. In that case, we're just left translating whatever the user chose to specify.

Also, take VRFMultispeedFan.idffor eg. You have one ZoneHVAC:TerminalUnit:VariableRefrigerantFlow (TU1) object with some COIL:Cooling:DX:VariableRefrigerantFlow and COIL:Heating:DX:VariableRefrigerantFlow. These coils do not have speed-related fields. Yet there is a UnitarySystemPerformance assigned to it.

I'm looking at VRFMultispeedFan.idf...

UnitarySystemPerformance:Multispeed, Sys 1 WaterToAirHP System MultiSpeed Performance, !- Name 2, !- Number of Speeds for Heating 2, !- Number of Speeds for Cooling

Where does the "2" come from? Should this value be hardcoded when coil object types are COIL:*:DX:VariableRefrigerantFlow?

That is arbitrary I believe.

jmarrec · 2023-09-12T13:34:19Z

src/energyplus/Test/ZoneHVACWaterToAirHeatPump_GTest.cpp

+    EXPECT_TRUE(idf_hp.isEmpty(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectType));
+    EXPECT_TRUE(idf_hp.isEmpty(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectName));


Please actually assign one and check that it's all good.

ci-commercialbuildings · 2023-09-26T23:10:20Z

CI Results for 73f1545:

Ubuntu 20.04:
Ubuntu 22.04:
- OpenStudio-3.7.0-alpha+bcab7d1154-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.7.0-alpha+bcab7d1154-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.7.0-alpha+ada77969dc-Darwin-x86_64.dmg
- OpenStudio-3.7.0-alpha+ada77969dc-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.7.0-alpha+8c529c60d8-Windows.exe
- OpenStudio-3.7.0-alpha+8c529c60d8-Windows.tar.gz
Code Coverage:

jmarrec

This is a tough one and I'm not sure I completely understand how this object is supposed to be used when not in a AirLoopHVAC:UnitarySystem. Maybe we should just punt on this new feature until a bit later after we've merged 23.2.0-IOFreeze to develop?

This does not enable using the object with VRF objects at least (eg, VRFMultispeedFan.idf is not reproducible). Seems like it'd work for the ZoneHVACWaterToAirHeatPump in most cases though.

For VRF we'd need to expose the Number of Speeds for <Heating|Cooling> fields nor the <Heating|Cooling> Speed <N> Supply Air Flow Ratio fields. Which may be problematic for other objects because that means we need to handle the case where what the user manually entered is inconsistent with the VSD coil's number of Speeds.

jmarrec · 2023-09-27T14:16:20Z

src/energyplus/ForwardTranslator/ForwardTranslateUnitarySystemPerformanceMultispeed.cpp

+    if (unitarySystems.size() == 1) {
+      AirLoopHVACUnitarySystem& unitarySystem = unitarySystems[0];
+      heatingCoil = unitarySystem.heatingCoil();
+      coolingCoil = unitarySystem.coolingCoil();
+    }
+
+    if (tuVRFs.size() == 1) {
+      ZoneHVACTerminalUnitVariableRefrigerantFlow& tuVRF = tuVRFs[0];
+      heatingCoil = tuVRF.heatingCoil();
+      coolingCoil = tuVRF.coolingCoil();
+    }
+
+    if (hps.size() == 1) {
+      ZoneHVACWaterToAirHeatPump& hp = hps[0];
+      heatingCoil = hp.heatingCoil().optionalCast<HVACComponent>();
+      coolingCoil = hp.coolingCoil().optionalCast<HVACComponent>();
+    }


Suggested change

if (unitarySystems.size() == 1) {

AirLoopHVACUnitarySystem& unitarySystem = unitarySystems[0];

heatingCoil = unitarySystem.heatingCoil();

coolingCoil = unitarySystem.coolingCoil();

}

if (tuVRFs.size() == 1) {

ZoneHVACTerminalUnitVariableRefrigerantFlow& tuVRF = tuVRFs[0];

heatingCoil = tuVRF.heatingCoil();

coolingCoil = tuVRF.coolingCoil();

}

if (hps.size() == 1) {

ZoneHVACWaterToAirHeatPump& hp = hps[0];

heatingCoil = hp.heatingCoil().optionalCast<HVACComponent>();

coolingCoil = hp.coolingCoil().optionalCast<HVACComponent>();

}

if (unitarySystems.size() == 1) {

AirLoopHVACUnitarySystem& unitarySystem = unitarySystems.front();

heatingCoil = unitarySystem.heatingCoil();

coolingCoil = unitarySystem.coolingCoil();

} else if (tuVRFs.size() == 1) {

ZoneHVACTerminalUnitVariableRefrigerantFlow& tuVRF = tuVRFs.front();

heatingCoil = tuVRF.heatingCoil();

coolingCoil = tuVRF.coolingCoil();

} else { // if (hps.size() == 1)

ZoneHVACWaterToAirHeatPump& hp = hps.front();

heatingCoil = hp.heatingCoil().optionalCast<HVACComponent>();

coolingCoil = hp.coolingCoil().optionalCast<HVACComponent>();

}

Because of the if (((unitarySystems.size() + tuVRFs.size() + hps.size()) != 1)) { line above, yes, I think we'd be able to collapse to your suggestion.

jmarrec · 2023-09-27T14:18:41Z

src/energyplus/ForwardTranslator/ForwardTranslateUnitarySystemPerformanceMultispeed.cpp

+      } else if (heatingCoil->iddObjectType() == openstudio::IddObjectType::OS_Coil_Heating_DX_VariableSpeed) {
+        auto heatingCoilDXVarspeed = heatingCoil->cast<CoilHeatingDXVariableSpeed>();
+        sysPerf.setInt(UnitarySystemPerformance_MultispeedFields::NumberofSpeedsforHeating, heatingCoilDXVarspeed.speeds().size());
+      } else if (heatingCoil->iddObjectType() == openstudio::IddObjectType::OS_Coil_Heating_WaterToAirHeatPump_VariableSpeedEquationFit) {
+        auto heatingCoilWaterToAirHeatingVar = heatingCoil->cast<CoilHeatingWaterToAirHeatPumpVariableSpeedEquationFit>();
+        sysPerf.setInt(UnitarySystemPerformance_MultispeedFields::NumberofSpeedsforHeating, heatingCoilWaterToAirHeatingVar.speeds().size());
+      } else {  // e.g., CoilHeatingWaterToAirHeatPumpEquationFit, CoilHeatingDXVariableRefrigerantFlow, CoilHeatingDXVariableRefrigerantFlowFluidTemperatureControl


If you run an AirLoopHVACUnitarySystem (or a ZoneHVACWAHP) with these coils and no UnitarySystemPerformance object, does E+ throw?

Just curious if you tested it, and same question for the below cooling coils below

jmarrec · 2023-09-27T14:23:45Z

...energyplus/ForwardTranslator/ForwardTranslateZoneHVACTerminalUnitVariableRefrigerantFlow.cpp

+    if (boost::optional<UnitarySystemPerformanceMultispeed> designSpecificationMultispeedObject = modelObject.designSpecificationMultispeedObject()) {
+      boost::optional<IdfObject> _unitarySystemPerformance = translateAndMapModelObject(designSpecificationMultispeedObject.get());
+
+      if (_unitarySystemPerformance && _unitarySystemPerformance->name()) {
+        idfObject.setString(ZoneHVAC_TerminalUnit_VariableRefrigerantFlowFields::DesignSpecificationMultispeedObjectType,
+                            _unitarySystemPerformance->iddObject().name());
+        idfObject.setString(ZoneHVAC_TerminalUnit_VariableRefrigerantFlowFields::DesignSpecificationMultispeedObjectName,
+                            _unitarySystemPerformance->name().get());
+      }
+    }


This is pointless here for the VRF object, because you don't have a way to set the number of Speeds during Heating/Cooling, and the VRF coils do not bear the requested information to autofill them.

Given what we see in VRFMultispeedFan.idf that means:

We actually need to expose the number of heating/cooling speeds

We also need the extensible portion <Heating|Cooling> Speed <N> Supply Air Flow Ratio

We probably need to throw or overwrite when writing for a coil that has an actual number of speeeds

We probably need to reach out to the E+ devs responsible for this feature to understand it fully. I'm quite fuzzy on how this is used with coils that don't have an actual number of speeds

Couple of things:
(1) I had noticed the "gap" on the heating/cooling speeds - that's why I had posed the question "Are we going to need to create a new setter method for speeds on the UnitarySystemPerformance object?" above.
(2) I thought you had said that the unitary system performance speeds in VRFMultispeedFan.idf were arbitrary?

But generally speaking, yes, I agree that we should probably punt on this new feature until later.

jmarrec · 2023-09-27T14:24:17Z

src/energyplus/ForwardTranslator/ForwardTranslateZoneHVACWaterToAirHeatPump.cpp

+    if (boost::optional<UnitarySystemPerformanceMultispeed> designSpecificationMultispeedObject = modelObject.designSpecificationMultispeedObject()) {
+      boost::optional<IdfObject> _unitarySystemPerformance = translateAndMapModelObject(designSpecificationMultispeedObject.get());
+
+      if (_unitarySystemPerformance && _unitarySystemPerformance->name()) {
+        idfObject.setString(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectType,
+                            _unitarySystemPerformance->iddObject().name());
+        idfObject.setString(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectName, _unitarySystemPerformance->name().get());
+      }
+    }


Unlike VRF, there are a varierty of coils when we can autofill the Number of Speeds for <Heating|Cooling> fields nor the <Heating|Cooling> Speed <N> Supply Air Flow Ratio.
One, or maybe the only one, where we cannot is the CoilHeatingWaterToAirHeatPumpEquationFit, and perhaps that's fine. I'm not 100% sure

joseph-robertson added 5 commits September 11, 2023 13:30

Add design spec multispeed fields to idd.

5cafd02

Update model files for zone hvac wtahp.

aab22b6

Update model files for zone hvac tu vrf.

3580d6f

Start to update ft and tests.

23f16fa

Formatting.

b0b9add

joseph-robertson added Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. IDDChange labels Sep 11, 2023

joseph-robertson added this to the OpenStudio SDK 3.7.0 milestone Sep 11, 2023

joseph-robertson self-assigned this Sep 11, 2023

joseph-robertson mentioned this pull request Sep 11, 2023

Update to EnergyPlus 23.2.0 #4934

Closed

26 tasks

jmarrec requested changes Sep 12, 2023

View reviewed changes

jmarrec and others added 19 commits September 12, 2023 15:34

Typo in ZoneHVACWaterToAirHeatPump

f7db3c1

Typo in idds and fix a clone method.

4cf55c5

Merge branch 'v23.2.0-IOFreeze' into design-spec-multispeed

f754205

Add model tests for both zone hvac.

d79407a

Add ft tests for checking for translated performance objects.

bb209bb

Check count of performance objects.

2e1fd84

Refactor unitarysysperf ft to support new zonehvac types.

d036c67

Formatting.

6f8f111

Update notes in idd.

546dd32

Support remaining coil types for sysperf ft.

8a869a1

Merge branch 'v23.2.0-IOFreeze' into design-spec-multispeed

0e48fed

Typos in sysperf ft.

268eb05

Merge branch 'v23.2.0-IOFreeze' into design-spec-multispeed

ff2593b

FT arbitrary 1 speed if coil object has no speeds.

2aa530f

Update new ft tests to check contents of sysperf.

ea58fd9

Get all includes for new ft tests.

98ccc33

Update new ft tests for noloadsupply values.

58b3c9b

Fix a few typos in sysperf ft.

c347caf

Move new ft tests into their own new sysperf ft test file.

592f486

joseph-robertson added 2 commits September 26, 2023 15:33

Formatting.

7cecbc9

Add more includes.

73f1545

joseph-robertson marked this pull request as ready for review September 26, 2023 23:28

joseph-robertson requested a review from jmarrec September 26, 2023 23:28

jmarrec requested changes Sep 27, 2023

View reviewed changes

Base automatically changed from v23.2.0-IOFreeze to develop September 28, 2023 06:54

DavidGoldwasser removed this from the OpenStudio SDK 3.7.0 milestone Oct 16, 2023

DavidGoldwasser marked this pull request as draft October 16, 2023 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZoneHVAC:*: new Design Specification Multispeed Object fields #4964

ZoneHVAC:*: new Design Specification Multispeed Object fields #4964

joseph-robertson commented Sep 11, 2023 •

edited

jmarrec Sep 12, 2023

jmarrec Sep 12, 2023

joseph-robertson Sep 12, 2023

jmarrec Sep 13, 2023

jmarrec Sep 13, 2023

joseph-robertson Sep 19, 2023

jmarrec Sep 25, 2023

joseph-robertson Sep 25, 2023

jmarrec Sep 26, 2023

jmarrec Sep 12, 2023

ci-commercialbuildings commented Sep 26, 2023 •

edited

jmarrec left a comment •

edited

jmarrec Sep 27, 2023

joseph-robertson Sep 27, 2023

jmarrec Sep 27, 2023

jmarrec Sep 27, 2023

joseph-robertson Sep 27, 2023

jmarrec Sep 27, 2023

		EXPECT_TRUE(idf_hp.isEmpty(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectType));
		EXPECT_TRUE(idf_hp.isEmpty(ZoneHVAC_WaterToAirHeatPumpFields::DesignSpecificationMultispeedObjectName));

ZoneHVAC:*: new Design Specification Multispeed Object fields #4964

Are you sure you want to change the base?

ZoneHVAC:*: new Design Specification Multispeed Object fields #4964

Conversation

joseph-robertson commented Sep 11, 2023 • edited

Pull request overview

Pull Request Author

Review Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Sep 26, 2023 • edited

jmarrec left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-robertson commented Sep 11, 2023 •

edited

ci-commercialbuildings commented Sep 26, 2023 •

edited

jmarrec left a comment •

edited