Fix #3905 - Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC by jmarrec · Pull Request #4253 · NatLabRockies/OpenStudio

jmarrec · 2021-03-22T12:51:09Z

Pull request overview

Fix E+ 9.3.0: Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC #3905
Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC
New regression test was added in #3905 - Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC OpenStudio-resources#137

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

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

…ld for supply fan (if on OASys or AIrLoopHVAC -> not required)

…oopHVAC, take first zone.

…opHVACOUtdoorAirSystem

…onents (this was affecting AirLoopHVACUnitarySystem too... but wasn't caught

…(not breaking public API yet)

… AirLoopHVACOutdoorAIrSystem

…oorAirSystem() getters

… in a smart way (not sure if I succeeded... I find it readable but maybe it's just me)

…ystem including testing the SPM Mixed air (failling for now)

…e that has the fan.

…asn't unique...)

jmarrec · 2021-03-22T15:42:24Z

resources/model/OpenStudio.idd

        \note Required for zone equipment. Leave blank if terminal unit is used in AirLoopHVAC:OutdoorAirSystem:EquipmentList.
        \note Also leave blank if terminal unit is used on main AirloopHVAC branch and terminal unit has no fan.
-  A7 ,  \field Supply Air Fan placement
+  A7 ,  \field Supply Air Fan Placement


This did not have any getter/setter until now. I renamed to match other model API objects, and E+

jmarrec · 2021-03-22T15:42:34Z

resources/model/OpenStudio.idd

+  A15; \field Controlling Zone or Thermostat Location
+       \type object-list
+       \object-list ThermalZoneNames
+       \note Used only for AirloopHVAC equipment on a main branch and defines zone name where thermostat is located.
+       \note Not required for zone equipment. Leave blank if terminal unit is used in AirLoopHVAC:OutdoorAirSystem:EquipmentList.
+       \note Required when terminal unit is used on main AirloopHVAC branch and coils are not set point controlled.
+       \note When terminal unit is used in air loop and is load controlled, this zone's thermostat will control operation.


jmarrec · 2021-03-22T15:43:14Z

resources/model/OpenStudio.idd

        \note This field is also ignored if VRF terminal unit is used on main AirloopHVAC branch
        \note and terminal unit has no fan.
  A8 ,  \field Supply Air Fan
-        \required-field


So, this is not required anymore... I have not changed the public model API for now because I would had to break backward compatibility. @kbenne

jmarrec · 2021-03-22T15:44:27Z

src/model/AirLoopHVACOutdoorAirSystem.cpp

+        } else if (auto comp = modelObject->optionalCast<ZoneHVACComponent>()) {
+          modelObjects.insert(modelObjects.begin(), *comp);
+          modelObject = comp->outletNode();


AirLoopHVACUnitarySystem also suffered from the same problem... It was not returned as part of AirLoopHVACOutdoorAirSystem_Impl::reliefComponents / AirLoopHVACOutdoorAirSystem_Impl::oaComponents

jmarrec · 2021-03-22T15:45:03Z

src/model/ZoneHVACTerminalUnitVariableRefrigerantFlow_Impl.hpp


      virtual unsigned outletPort() const override;

+      virtual bool addToNode(Node& node) override;


Override to allow connecting to AirLoopHVAC/AirLoopHVACOutdoorAirSystem

jmarrec · 2021-03-22T15:57:37Z

src/model/test/ZoneHVACTerminalUnitVariableRefrigerantFlow_GTest.cpp

+  if (boost::optional<Node> OANode = outdoorAirSystem.outboardOANode()) {
+    ZoneHVACTerminalUnitVariableRefrigerantFlow testObject(m);
+
+    EXPECT_TRUE(testObject.addToNode(OANode.get()));
+    EXPECT_EQ(3, airLoop.supplyComponents().size());
+    EXPECT_EQ(3, outdoorAirSystem.oaComponents().size());
+    EXPECT_TRUE(testObject.airLoopHVACOutdoorAirSystem());
+  }
+
+  if (boost::optional<Node> reliefNode = outdoorAirSystem.outboardReliefNode()) {
+    ZoneHVACTerminalUnitVariableRefrigerantFlow testObject(m);
+    EXPECT_TRUE(testObject.addToNode(reliefNode.get()));
+    EXPECT_EQ(3, airLoop.supplyComponents().size());
+    EXPECT_EQ(3, outdoorAirSystem.reliefComponents().size());
+    EXPECT_TRUE(testObject.airLoopHVACOutdoorAirSystem());
+  }


Test on AirLoopHVACOutdoorAirSystem: both OA intake and relief

jmarrec · 2021-03-22T15:57:51Z

src/model/test/ZoneHVACTerminalUnitVariableRefrigerantFlow_GTest.cpp

+  }
+}
+
+TEST_F(ModelFixture, ZoneHVACTerminalUnitVariableRefrigerantFlow_controllingZoneorThermostatLocation) {


Test this field

jmarrec · 2021-03-22T15:58:01Z

src/model/test/ZoneHVACTerminalUnitVariableRefrigerantFlow_GTest.cpp

+  EXPECT_FALSE(testObject.controllingZoneorThermostatLocation());
+}
+
+TEST_F(ModelFixture, ZoneHVACTerminalUnitVariableRefrigerantFlow_SupplyAirFanPlacement) {


And the other new one, including checking that it's correctly defaulted in both Ctors to "DrawThrough" (got bit by it in Reg test originally)

jmarrec · 2021-03-22T15:58:59Z

src/energyplus/Test/ZoneHVACTerminalUnitVariableRefrigerantFlow_GTest.cpp

    EXPECT_EQ(idf_fan.getString(Fan_OnOffFields::AirOutletNodeName).get(), idf_supHC.getString(Coil_Heating_WaterFields::AirInletNodeName).get());
  }
+
+  // Supplemental HC = CoilHeatingWater, BlowThrough


New FT test case to check that in BlowThrough, the nodes are also connected properly.

jmarrec · 2021-03-22T16:00:04Z

src/energyplus/Test/ZoneHVACTerminalUnitVariableRefrigerantFlow_GTest.cpp

+  WorkspaceObjectVector spm_mixedairs(w.getObjectsByType(IddObjectType::SetpointManager_MixedAir));
+  EXPECT_EQ(2u, spm_mixedairs.size());
+  for (const auto& spm_mixedair : spm_mixedairs) {
+    EXPECT_EQ("Temperature", spm_mixedair.getString(SetpointManager_MixedAirFields::ControlVariable).get());
+    EXPECT_EQ("Supply Outlet Node", spm_mixedair.getString(SetpointManager_MixedAirFields::ReferenceSetpointNodeName).get());
+    EXPECT_EQ(vrf.nameString() + " Heating Coil Outlet Node", spm_mixedair.getString(SetpointManager_MixedAirFields::FanInletNodeName).get());
+    EXPECT_EQ("Supply Outlet Node", spm_mixedair.getString(SetpointManager_MixedAirFields::FanOutletNodeName).get());
+    EXPECT_TRUE(spm_mixedair.nameString().find("OS Default SPM") != std::string::npos);
+    if (spm_mixedair.nameString().find("Mixed Air") != std::string::npos) {
+      EXPECT_EQ("Mixed Air Node", spm_mixedair.getString(SetpointManager_MixedAirFields::SetpointNodeorNodeListName).get());
+    } else {
+      EXPECT_EQ("VRF TU on Outdoor Air Outlet Node", spm_mixedair.getString(SetpointManager_MixedAirFields::SetpointNodeorNodeListName).get());
+    }
+  }


Check that SPM:MixedAir correctly picked up the fan that is inside the VRF TU on the main branch

…rSystem, or placed on AirLoopHVAC main branch that has its own supply fan

jmarrec · 2021-03-23T15:09:10Z

Windows CI is reporting the usual RUby test failure: 2998 - RubyTest-Date_Test-ymd_constructor (Failed)

...energyplus/ForwardTranslator/ForwardTranslateZoneHVACTerminalUnitVariableRefrigerantFlow.cpp

…nalUnitVariableRefrigerantFlow.cpp

ci-commercialbuildings · 2021-03-23T16:07:20Z

CI Results for 5415cc9:

jmarrec added 16 commits March 22, 2021 13:49

#3905 - Add new field Controlling Zone for VRFTU, remove required-fie…

46b0e12

…ld for supply fan (if on OASys or AIrLoopHVAC -> not required)

Implement the new field Controlling Thermostat for VRF TU

682b596

FT For Control Zone: If filled out use that. If not and if on an AirL…

7a72cc3

…oopHVAC, take first zone.

Override addToNode for VRF TU to allow placing on AirLoopHVAC / AirLo…

4f10db1

…opHVACOUtdoorAirSystem

Model tests + one fix for AirLoopHVACOutdoorAirSystem::oa/refliefComp…

292cece

…onents (this was affecting AirLoopHVACUnitarySystem too... but wasn't caught

Make supplyAirFan return OptionalHVACComponent on the Impl side only …

17622b0

…(not breaking public API yet)

Do not translate OA Mixer for the VRF TU if it's on an AirLoopHVAC or…

deaf98b

… AirLoopHVACOutdoorAIrSystem

Improve testing by checking the VRFTU airLoopHVAC() / airLoopHVACOutd…

59b7e93

…oorAirSystem() getters

Implement Supply Air Fan Placement for VRF in model

f388610

Update FT to handle DrawThrough versus BlowThrough, trying to do that…

bc4819b

… in a smart way (not sure if I succeeded... I find it readable but maybe it's just me)

Missed one Ctor to set the value of Fan Placement

0223612

Test that DrawThrough is correctly set as default in both model Ctors

3bf2666

FT test in BlowThrough mode

6d594ee

Full FT test for placing VRF TU on AirLoopHVAC/AirLoopHVACOutdoorAirS…

ef0bee1

…ystem including testing the SPM Mixed air (failling for now)

Correctly set the SPM Mixed Air "Fan" names when the VRF TU is the on…

222facd

…e that has the fan.

Caught a typo during OpenStudio-resources reg test (fan outlet node w…

d94ac9b

…asn't unique...)

jmarrec self-assigned this Mar 22, 2021

jmarrec added component - IDF Translation component - Model Enhancement Request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Mar 22, 2021

jmarrec mentioned this pull request Mar 22, 2021

#3905 - Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC NatLabRockies/OpenStudio-resources#137

Merged

27 tasks

jmarrec requested a review from kbenne March 22, 2021 15:41

jmarrec commented Mar 22, 2021

View reviewed changes

Add VT for defaulting Fan Placement to DrawThrough + VT test

aecc34e

jmarrec added this to the OpenStudio SDK 3.2.0 milestone Mar 22, 2021

jmarrec added the Ready for Merge Set this once CI is passing and reviews are approved. label Mar 22, 2021

Do not translate Fan inside VRF TU if: placed on AirLoopHVACOutdoorAi…

1a5c1d5

…rSystem, or placed on AirLoopHVAC main branch that has its own supply fan

jmarrec commented Mar 23, 2021

View reviewed changes

...energyplus/ForwardTranslator/ForwardTranslateZoneHVACTerminalUnitVariableRefrigerantFlow.cpp Outdated Show resolved Hide resolved

Update src/energyplus/ForwardTranslator/ForwardTranslateZoneHVACTermi…

5415cc9

…nalUnitVariableRefrigerantFlow.cpp

This was referenced Mar 28, 2021

Empty AirLoopHVAC:ControllerList created for AirLoopHVAC:DedicatedOutdoorAirSystem at forward translation #4265

Closed

Issues with VRF Terminal Unit on AirLoopHVAC:DedicatedOutdoorAirSystem NatLabRockies/EnergyPlus#8676

Open

tijcolem merged commit 063dea9 into develop Apr 6, 2021

tijcolem deleted the 3905_VRF_Terminal_AirLoopHVAC branch April 6, 2021 01:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3905 - Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC#4253

Fix #3905 - Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to connect to AirLoopHVAC#4253
tijcolem merged 19 commits intodevelopfrom
3905_VRF_Terminal_AirLoopHVAC

jmarrec commented Mar 22, 2021 •

edited

Loading

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec Mar 22, 2021

Uh oh!

jmarrec commented Mar 23, 2021

Uh oh!

Uh oh!

ci-commercialbuildings commented Mar 23, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		virtual unsigned outletPort() const override;

		virtual bool addToNode(Node& node) override;

Conversation

jmarrec commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Mar 23, 2021

Uh oh!

Uh oh!

ci-commercialbuildings commented Mar 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmarrec commented Mar 22, 2021 •

edited

Loading

ci-commercialbuildings commented Mar 23, 2021 •

edited

Loading