Support CoilSystem:Cooling:Water by joseph-robertson · Pull Request #5350 · NatLabRockies/OpenStudio

joseph-robertson · 2025-01-31T22:05:24Z

Pull request overview

Fixes E+ 9.6.0: Wrap CoilSystem:Cooling:Water #4408
Fixes ControllerWaterCoil not connected to CoilCoolingWater within CoilSystemCoolingWaterHeatExchangerAssisted if coil added to plant loop before coil system added to air loop #4895

Wrap CoilSystem:Cooling:Water, and support:

"Water Size Economizer Model" (WaterSideEconomizer_PreCoolCoil.idf)
"Wrap Around Water Coil Heat Recovery Mode" (5ZoneAirCooled_RunaroundHeatRecovery.idf).

Related OpenStudio-resources tests:

airterminal_cooledbeam.rb (CoilCoolingWater)
coilsystem_waterhx.rb (CoilSystemCoolingWaterHeatExchangerAssisted)
coilsystem_cooling_water.rb (CoilSystemCoolingWater) <- new test file

Perhaps we don't need to create a Controller:WaterCoil in coil.addToNode.

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: Support CoilSystem:Cooling:Water OpenStudio-resources#218
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

… and coil system.

jmarrec · 2025-03-14T08:52:57Z

@joseph-robertson this is still marked as draft, no review asked, what is the status of this?

and is there a corresponding OS-resources test?

… and coil system.

jmarrec

Reviewed IDD and Ctors, good!

…companion to CoilCoolingWater per IDD/IO ref CoilSystem:Cooling:Water:HeatExchangerAssisted isn't WaterToAirComponent

…t ctor to remove() and LOG_AND_THROW

jmarrec · 2025-05-05T08:40:50Z

+
+    boost::optional<HVACComponent> CoilSystemCoolingWater_Impl::containingHVACComponent() const {
+      // AirLoopHVACUnitarySystem
+      std::vector<AirLoopHVACUnitarySystem> airLoopHVACUnitarySystems = this->model().getConcreteModelObjects<AirLoopHVACUnitarySystem>();
+
+      for (const auto& airLoopHVACUnitarySystem : airLoopHVACUnitarySystems) {
+        if (boost::optional<HVACComponent> coolingCoil = airLoopHVACUnitarySystem.coolingCoil()) {
+          if (coolingCoil->handle() == this->handle()) {
+            return airLoopHVACUnitarySystem;
+          }
+        }
+      }
+
+      return boost::none;
+    }


Can this really be added to a AirLoopHVACUnitarySystem?

Our model API is ridden with this weird stuff, but AirLoopHVACUnitarySystem IS a ZoneHVACComponent

https://github.com/NREL/OpenStudio/blob/4c3a4a3f83065e84278f9a6c1e2f64b2c0a0ac36/src/model/AirLoopHVACUnitarySystem.hpp#L30

AFAIK, this cannot be added to any HVAC/ZoneHVACComponent. The E+ IDD shows no possible references. I'm going to remove this.

14c7357

jmarrec · 2025-05-05T09:24:12Z

+  // Cooling Coil: Required Object
+  CoilCoolingWater coolingCoil(m);


Nowhere I'm seeing a test with the other coil type: CoilSystem:Cooling:Water:HeatExchangerAssisted

And I'm realizing that this object isn't a WaterToAirComponent, so your implementation is likely broken.

Done in #5404

jmarrec · 2025-05-05T09:39:59Z

+          // Shouldn't happen, accepts only Coil:Cooling:Water or Coil:Cooling:Water:DetailedGeometry
+          // Shouldn't happen, accepts only Coil:Cooling:DX:SingleSpeed or Coil:Cooling:DX:VariableSpeed


first line is wrong, second is a copy paste

jmarrec · 2025-05-05T09:40:28Z

+        idfObject.setString(CoilSystem_Cooling_WaterFields::CompanionCoilUsedForHeatRecovery, wo_->nameString());
+        if (wo_->iddObject().type() == IddObjectType::Coil_Cooling_Water) {
+          wo_->setString(Coil_Cooling_WaterFields::AirInletNodeName, modelObject.airLoopHVACOutdoorAirSystem()->reliefAirModelObject()->nameString());
+          wo_->setString(Coil_Cooling_WaterFields::AirOutletNodeName, wo_->nameString() + " Exhaust Outlet Node");  // FIXME


jmarrec · 2025-05-05T09:55:45Z

+    ok = setCoolingCoil(coolingCoil);
+    OS_ASSERT(ok);


TODO: Need to gracefully handle this instead of hitting a boost assert!

Unlike the rest of the setXXX calls, this is likely to fail, in case the user passed a wrong HVACComponent type, so we need to remove() and LOG_AND_THROW

Done in #5404

jmarrec · 2025-05-05T10:25:42Z

+
+    EXPECT_EQ(2u, a.supplyComponents().size());
+
+    EXPECT_TRUE(cc.addToNode(n));


I misread since it's truncated. cc is not the CoilSystemCoolingater but the child CoilCoolingWater here. No, this should not be allowed.

…lSystemCoolingWater

…nnect the water side for the children coils

… of the primary cooling cooling of a CoilSystemCoolingWater to an AirLoopHVAC Ensures coils that are part of a cooling system are not directly connected to an air loop, preventing potential errors. Also manages the creation and removal of ControllerWaterCoil objects based on the coil's parent system and presence within a ZoneHVACComponent.

…wed anywhere in a ZoneHVAC/HVACComponent

…t (not supported AFAIK)

jmarrec · 2025-05-05T12:16:59Z

+      // CoilSystemCoolingWater
+      {
+        auto coilSystems = model().getConcreteModelObjects<CoilSystemCoolingWater>();
+        for (const auto& coilSystem : coilSystems) {
+          if (coilSystem.coolingCoil().handle() == handle()) {
+            return coilSystem;
+          }
+          if (boost::optional<WaterToAirComponent> companionCoilUsedForHeatRecovery = coilSystem.companionCoilUsedForHeatRecovery()) {
+            if (companionCoilUsedForHeatRecovery->handle() == this->handle()) {
+              return coilSystem;
+            }
+          }
+        }
+      }


Need the same for CoilSystemCoolingWaterHeatExchangerAssisted

Done in #5404

jmarrec · 2025-05-05T14:01:17Z

+    // Special case for CoilSystemCoolingWater (Wrap Around Water Coil Heat Recovery Mode)
+    for (const auto& comp : subsetCastVector<HVACComponent>(plantLoop.demandComponents())) {
+      if (comp.iddObject().type().value() == openstudio::IddObjectType::OS_Coil_Cooling_Water) {
+        auto coil = comp.cast<CoilCoolingWater>();
+        if (auto containingHVACComponent = coil.containingHVACComponent()) {
+          if (auto coilSystemCoolingWater = containingHVACComponent->optionalCast<CoilSystemCoolingWater>()) {
+            result.push_back(operationSchemeComponent(*coilSystemCoolingWater));
+          }
+        }
+      }
+    }
+


Hummmm. I am NOT following this one... @joseph-robertson

…ode (and getting them out of sync) in both ctors

…has a FIXME (it fails) /home/julien/Software/Others/OpenStudio/src/energyplus/Test/CoilSystemCoolingWater_GTest.cpp:297: Failure Expected equality of these values: "Relief Node" idf_companionCoil.getString(Coil_Cooling_WaterFields::AirOutletNodeName).get() Which is: "CompanionCoilUsedForHeatRecovery Exhaust Outlet Node"

…oilSystemWater

#5350 - Changes to Support CoilSystem:Cooling:Water

…ded (and OS-resources test show E+ issuing a warning)

ci-commercialbuildings · 2025-05-05T20:28:17Z

CI Results for be13f9f:

Ubuntu 24.04:
- OpenStudio-3.10.0-alpha+be13f9f3fe-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.10.0-alpha+be13f9f3fe-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
- OpenStudio-3.10.0-alpha+be13f9f3fe-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.10.0-alpha+be13f9f3fe-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.10.0-alpha+be13f9f3fe-Darwin-x86_64.dmg
- OpenStudio-3.10.0-alpha+be13f9f3fe-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.10.0-alpha+be13f9f3fe-Windows.exe
- OpenStudio-3.10.0-alpha+be13f9f3fe-Windows.tar.gz
Code Coverage:

joseph-robertson added 2 commits January 31, 2025 15:03

Separate coil cooling water ft into without unitary and coil system.

0fbc857

Stub ft test for coil cooling water.

22abca8

joseph-robertson added Enhancement Request component - Model component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Jan 31, 2025

joseph-robertson self-assigned this Jan 31, 2025

joseph-robertson added 12 commits January 31, 2025 15:19

Few typos in new ft method.

9c21359

Expand new ft test.

f29ca2e

Typo.

beff059

Fix ft test.

43e8165

Separate coilsystem cooling water hx assisted ft into without unitary…

68870dc

… and coil system.

Formatting.

8dfc486

Typo in cmakelists.

b3235b7

Few typos in new ft.

7ba0733

Update new ft.

2a7b8c3

Another typo in ft.

d974d09

Special case for coilsystem ft on plant loop branch.

45254a6

Fix and update ft tests.

8d6f69c

joseph-robertson added 10 commits March 14, 2025 09:53

Separate coil cooling water ft into without unitary and coil system.

a6e0711

Stub ft test for coil cooling water.

d63b3f1

Few typos in new ft method.

5b02c72

Expand new ft test.

f2a8287

Typo.

25ab778

Fix ft test.

c8248d6

Separate coilsystem cooling water hx assisted ft into without unitary…

6d65d77

… and coil system.

Formatting.

63303d3

Typo in cmakelists.

74b017a

Few typos in new ft.

46da7e4

Some cleanup.

39d8693

joseph-robertson marked this pull request as ready for review April 30, 2025 15:43

joseph-robertson requested a review from jmarrec April 30, 2025 15:44

Merge branch 'develop' into coilsystem-cooling-water

4c3a4a3

jmarrec reviewed May 5, 2025

View reviewed changes

jmarrec added 3 commits May 5, 2025 11:44

Make the coolingCoil/companion return a HVACComponent + restrict the …

041adbd

…companion to CoilCoolingWater per IDD/IO ref CoilSystem:Cooling:Water:HeatExchangerAssisted isn't WaterToAirComponent

Improve model gtests

310a14f

Test Explicit Ctor, including wrong cooling coil type + fixup explici…

d0cc34b

…t ctor to remove() and LOG_AND_THROW

jmarrec reviewed May 5, 2025

View reviewed changes

jmarrec added 7 commits May 5, 2025 14:50

Remove useless optionalCompanionCoilUsedForHeatRecovery

ba57cda

CoilSystemCoolingWaterHXAssisted:containingHVACComponent picks up Coi…

4f3e2d4

…lSystemCoolingWater

Extend testing to ensure that remove works in particular

63acda6

Override remove() for CoilSystemCoolingWaterXX objects: need to disco…

b264a56

…nnect the water side for the children coils

I have no reason to believe that CoilSystemCoolingWater would be allo…

14c7357

…wed anywhere in a ZoneHVAC/HVACComponent

Adjust FT test after changing return type, and remove the Unitary tes…

95c4727

…t (not supported AFAIK)

jmarrec mentioned this pull request May 5, 2025

#5350 - Changes to Support CoilSystem:Cooling:Water #5404

Merged

19 tasks

Remove unneeded bit in ForwardTranslateAirLoopHVAC

3d6d145

jmarrec reviewed May 5, 2025

View reviewed changes

jmarrec added 7 commits May 5, 2025 16:20

Add a private assignEnergyPlusIDDDefaults method to avoid repeating c…

868633a

…ode (and getting them out of sync) in both ctors

Fixup the FIXME

51a0ae3

Add a FT Gtest for a Case with a CoilSystemWaterHXAssisted inside a C…

61ea54d

…oilSystemWater

Fixup FT to handle CoilSystemWaterHXAssisted inside CoilSystemWater

e1705d9

Merge pull request #5404 from NREL/coilsystem-cooling-water2

97e0142

#5350 - Changes to Support CoilSystem:Cooling:Water

Remove copy pasted bit that tries to add a SPM:MixedAir: it's not nee…

be13f9f

…ded (and OS-resources test show E+ issuing a warning)

joseph-robertson merged commit 83cec57 into develop May 7, 2025

joseph-robertson deleted the coilsystem-cooling-water branch May 7, 2025 16:19

jmarrec mentioned this pull request Jul 7, 2025

[OS SDK 3.10.0] Add CoilSystem:Cooling:Water openstudiocoalition/OpenStudioApplication#817

Open

		// Cooling Coil: Required Object
		CoilCoolingWater coolingCoil(m);

		// Shouldn't happen, accepts only Coil:Cooling:Water or Coil:Cooling:Water:DetailedGeometry
		// Shouldn't happen, accepts only Coil:Cooling:DX:SingleSpeed or Coil:Cooling:DX:VariableSpeed


		EXPECT_EQ(2u, a.supplyComponents().size());

		EXPECT_TRUE(cc.addToNode(n));

Uh oh!

Conversation

joseph-robertson commented Jan 31, 2025 • 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!

jmarrec commented Mar 14, 2025

Uh oh!

jmarrec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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 May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ci-commercialbuildings commented May 5, 2025 • 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!

5 participants

joseph-robertson commented Jan 31, 2025 •

edited

Loading

jmarrec May 5, 2025 •

edited

Loading

ci-commercialbuildings commented May 5, 2025 •

edited

Loading