Merge pull request #4050 from NREL/3921_RefrigerationSystem_Children

Fix #3921 - some children of RefrigerationSystem can be added several times and produce a fatal when System is removed

developer/doc/ReleaseNotes/OpenStudio_Release_Notes_3_1_0.md

@@ -66,6 +66,8 @@ A number of API-breaking changes have been implemented in OpenStudio 3.1.0:
 * [#3960](https://github.com/NREL/OpenStudio/pull/3960) - Added support for building the C# bindings via `dotnet` CLI, including on Unix platforms.
 * [#3959](https://github.com/NREL/OpenStudio/pull/3959) - Also included some improvements in the generated C# bindings by reducing build warnings and properly exposing some types via SWIG
     * `ScheduleTypeKey` (which is normally only use by OpenStudio internals in ScheduleTypeRegistry checks) previously mapped to a `std::pair<std::string, std::string>` which was SWIGed in ruby as an Array of strings of size two, but improperly exposed in C#. It now uses a dedicated helper class with two methods `ScheduleTypeKey::className()` and `ScheduleTypeKey::scheduleDisplayName()`
+* [#4050](https://github.com/NREL/OpenStudio/pull/4050) - Fix #3921 - some children of RefrigerationSystem can be added several times and produce a fatal when System is removed.
+    * `RefrigerationSystem` will now be enforcing unicity for all children. This was already the case for a few of the child objects such as Case and WalkIns, it is now the case for all objects (`RefrigerationCondenserCascade`, `RefrigerationSubcoolerMechanical` and `RefrigerationSubcoolerLiquidSuction`, etc.). What this means is that setter methods (or add methods in case of a list) will remove the child for any current RefrigerationSystem it is on first.
 * [#4066](https://github.com/NREL/OpenStudio/pull/4066) - Multiple shading controls referenced by a single subsurface
     * `SubSurface` was historically the one referencing the `ShadingControl` object. Now it's a many-to-many relationship where `ShadingControl` has an extensible 'Sub Surface Name' field, and multiple `ShadingControl` objects can reference the same `SubSurface`. This is trickling down from a change introduced in EnergyPlus version 9.4, and specifically in PR [NREL/EnergyPlus#8196](https://github.com/NREL/EnergyPlus/pull/8196)
     * Methods in `SubSurface` have been deprecated but are kept for backward compatibility. They will be removed in a future version of OpenStudio:

resources/model/OpenStudio.idd

@@ -26381,6 +26381,7 @@ OS:WaterHeater:HeatPump,
   N2 , \field Condenser Water Flow Rate
        \type real
        \units m3/s
+       \ip-units gal/min
        \minimum> 0
        \autosizable
        \note Actual water flow rate through the heat pump's water coil (condenser).
@@ -26445,9 +26446,16 @@ OS:WaterHeater:HeatPump,
        \type real
        \units C
        \required-field
-       \minimum -5
        \note Heat pump compressor will not operate when the inlet air dry-bulb temperature
        \note is below this value.
+  N5 , \field Maximum Inlet Air Temperature for Compressor Operation
+       \type real
+       \units C
+       \required-field
+       \minimum 26
+       \maximum 94
+       \note Heat pump compressor will not operate when the inlet air dry-bulb temperature
+       \note is above this value.
   A13, \field Compressor Location
        \required-field
        \type choice
@@ -26476,14 +26484,14 @@ OS:WaterHeater:HeatPump,
        \required-field
        \note BlowThrough means the fan is located before the air coil (upstream).
        \note DrawThrough means the fan is located after the air coil (downstream).
-  N5 , \field On Cycle Parasitic Electric Load
+  N6 , \field On Cycle Parasitic Electric Load
        \type real
        \units W
        \minimum 0.0
        \required-field
        \note Parasitic electric power consumed when the heat pump compressor operates.
        \note Does not contribute to water heating but can impact the zone air heat balance.
-  N6 , \field Off Cycle Parasitic Electric Load
+  N7 , \field Off Cycle Parasitic Electric Load
        \type real
        \units W
        \minimum 0.0
@@ -26875,7 +26883,6 @@ OS:WaterHeater:HeatPump:WrappedCondenser,
   N5 , \field Minimum Inlet Air Temperature for Compressor Operation
        \type real
        \units C
-       \minimum -5
        \required-field
        \note Heat pump compressor will not operate when the inlet air dry-bulb temperature
        \note is below this value.

src/energyplus/ForwardTranslator/ForwardTranslateWaterHeaterHeatPump.cpp

@@ -124,6 +124,11 @@ boost::optional<IdfObject> ForwardTranslator::translateWaterHeaterHeatPump(
     idfObject.setDouble(WaterHeater_HeatPump_PumpedCondenserFields::MinimumInletAirTemperatureforCompressorOperation,value);
   }
 
+  {
+    auto value = modelObject.maximumInletAirTemperatureforCompressorOperation();
+    idfObject.setDouble(WaterHeater_HeatPump_PumpedCondenserFields::MaximumInletAirTemperatureforCompressorOperation,value);
+  }
+
   idfObject.setDouble(WaterHeater_HeatPump_PumpedCondenserFields::MaximumInletAirTemperatureforCompressorOperation,94);
 
   {

src/energyplus/ForwardTranslator/ForwardTranslateWaterHeaterStratified.cpp

@@ -523,10 +523,8 @@ boost::optional<IdfObject> ForwardTranslator::translateWaterHeaterStratified( Wa
   }
 
   // Number of Nodes
-  {
-    auto num_nodes = modelObject.numberofNodes();
-    idfObject.setInt(WaterHeater_StratifiedFields::NumberofNodes,num_nodes);
-  }
+  auto num_nodes = modelObject.numberofNodes();
+  idfObject.setInt(WaterHeater_StratifiedFields::NumberofNodes,num_nodes);
 
   // Additional Destratification Conductivity
   value = modelObject.additionalDestratificationConductivity();
@@ -537,84 +535,84 @@ boost::optional<IdfObject> ForwardTranslator::translateWaterHeaterStratified( Wa
 
   // Node 1 Additional Loss Coefficient
   value = modelObject.node1AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 1))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node1AdditionalLossCoefficient,value.get());
   }
 
   // Node 2 Additional Loss Coefficient
   value = modelObject.node2AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 2))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node2AdditionalLossCoefficient,value.get());
   }
 
   // Node 3 Additional Loss Coefficient
   value = modelObject.node3AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 3))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node3AdditionalLossCoefficient,value.get());
   }
 
   // Node 4 Additional Loss Coefficient
   value = modelObject.node4AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 4))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node4AdditionalLossCoefficient,value.get());
   }
 
   // Node 5 Additional Loss Coefficient
   value = modelObject.node5AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 5))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node5AdditionalLossCoefficient,value.get());
   }
 
   // Node 6 Additional Loss Coefficient
   value = modelObject.node6AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 6))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node6AdditionalLossCoefficient,value.get());
   }
 
   // Node 7 Additional Loss Coefficient
   value = modelObject.node7AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 7))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node7AdditionalLossCoefficient,value.get());
   }
 
   // Node 8 Additional Loss Coefficient
   value = modelObject.node8AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 8))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node8AdditionalLossCoefficient,value.get());
   }
 
   // Node 9 Additional Loss Coefficient
   value = modelObject.node9AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 9))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node9AdditionalLossCoefficient,value.get());
   }
 
   // Node 10 Additional Loss Coefficient
   value = modelObject.node10AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 10))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node10AdditionalLossCoefficient,value.get());
   }
 
   // Node 11 Additional Loss Coefficient
   value = modelObject.node11AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 11))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node11AdditionalLossCoefficient,value.get());
   }
 
   // Node 12 Additional Loss Coefficient
   value = modelObject.node12AdditionalLossCoefficient();
-  if( value )
+  if (value && (num_nodes >= 12))
   {
     idfObject.setDouble(WaterHeater_StratifiedFields::Node12AdditionalLossCoefficient,value.get());
   }

src/energyplus/Test/OutputDiagnostics_GTest.cpp

@@ -108,7 +108,7 @@ TEST_F(EnergyPlusFixture, ReverseTranslator_OutputDiagnostics) {
                                   // Oops, this one is twice
                                   "ReportDuringHVACSizingSimulation", "ReportDuringHVACSizingSimulation"});
 
-    for (const auto key: keys) {
+    for (const auto& key: keys) {
       IdfExtensibleGroup eg = _i_outputDiagnostics->pushExtensibleGroup();
       EXPECT_TRUE(eg.setString(Output_DiagnosticsExtensibleFields::Key, key));
     }

src/model/CMakeLists.txt

@@ -2089,6 +2089,7 @@ set(${target_name}_test_src
   test/UtilityBill_GTest.cpp
   test/WaterHeaterMixed_GTest.cpp
   test/WaterHeaterHeatPump_GTest.cpp
+  test/WaterHeaterHeatPumpWrappedCondenser_GTest.cpp
   test/WaterHeaterStratified_GTest.cpp
   test/WaterUseConnections_GTest.cpp
   test/WaterUseEquipment_GTest.cpp

src/model/RefrigerationCompressor.cpp

@@ -107,7 +107,7 @@ namespace detail {
   std::vector<IdfObject> RefrigerationCompressor_Impl::remove()
   {
     // Remove from ModelObjectList(s) in RefrigerationSystem(s) if needed
-    this->removeFromSystems();
+    this->removeFromSystem();
 
     return ParentObject_Impl::remove();
   }
@@ -353,19 +353,19 @@ namespace detail {
     return getObject<ModelObject>().getModelObjectTarget<CurveBicubic>(OS_Refrigeration_CompressorFields::RefrigerationCompressorCapacityCurveName);
   }
 
-  std::vector<RefrigerationSystem> RefrigerationCompressor_Impl::systems() const {
-    std::vector<RefrigerationSystem> result;
+  boost::optional<RefrigerationSystem> RefrigerationCompressor_Impl::system() const {
+    boost::optional<RefrigerationSystem> result;
 
     RefrigerationCompressor refrigerationCompressor = this->getObject<RefrigerationCompressor>();
     for (RefrigerationSystem refrigerationSystem : this->model().getConcreteModelObjects<RefrigerationSystem>()) {
       RefrigerationCompressorVector refrigerationCompressors = refrigerationSystem.compressors();
       if ( !refrigerationCompressors.empty() && std::find(refrigerationCompressors.begin(), refrigerationCompressors.end(), refrigerationCompressor) != refrigerationCompressors.end() ) {
-        result.push_back(refrigerationSystem);
+        result = refrigerationSystem;
         break;
       } else {
         RefrigerationCompressorVector refrigerationHighStageCompressors = refrigerationSystem.highStageCompressors();
         if ( !refrigerationHighStageCompressors.empty() && std::find(refrigerationHighStageCompressors.begin(), refrigerationHighStageCompressors.end(), refrigerationCompressor) != refrigerationHighStageCompressors.end() ) {
-          result.push_back(refrigerationSystem);
+          result = refrigerationSystem;
           break;
         }
       }
@@ -374,12 +374,11 @@ namespace detail {
     return result;
   }
 
-  void RefrigerationCompressor_Impl::removeFromSystems() {
-    RefrigerationCompressor refrigerationCompressor = this->getObject<RefrigerationCompressor>();
-
-    for (RefrigerationSystem& refrigerationSystem: this->systems()) {
-      refrigerationSystem.removeCompressor(refrigerationCompressor);
-      refrigerationSystem.removeHighStageCompressor(refrigerationCompressor);
+  void RefrigerationCompressor_Impl::removeFromSystem() {
+    if (boost::optional<RefrigerationSystem> refrigerationSystem = this->system()) {
+      RefrigerationCompressor refrigerationCompressor = this->getObject<RefrigerationCompressor>();
+      refrigerationSystem->removeCompressor(refrigerationCompressor);
+      refrigerationSystem->removeHighStageCompressor(refrigerationCompressor);
     }
   }
 
@@ -569,12 +568,12 @@ void RefrigerationCompressor::resetTranscriticalCompressorCapacityCurve() {
   getImpl<detail::RefrigerationCompressor_Impl>()->resetTranscriticalCompressorCapacityCurve();
 }
 
-std::vector<RefrigerationSystem> RefrigerationCompressor::systems() const {
-  return getImpl<detail::RefrigerationCompressor_Impl>()->systems();
+boost::optional<RefrigerationSystem> RefrigerationCompressor::system() const {
+  return getImpl<detail::RefrigerationCompressor_Impl>()->system();
 }
 
-void RefrigerationCompressor::removeFromSystems() {
-  getImpl<detail::RefrigerationCompressor_Impl>()->removeFromSystems();
+void RefrigerationCompressor::removeFromSystem() {
+  getImpl<detail::RefrigerationCompressor_Impl>()->removeFromSystem();
 }
 
 /// @cond

src/model/RefrigerationCompressor.hpp

@@ -89,8 +89,8 @@ class MODEL_API RefrigerationCompressor : public ParentObject {
 
   boost::optional<CurveBicubic> transcriticalCompressorCapacityCurve() const;
 
-  // Returns the parent RefrigerationSystem(s)
-  std::vector<RefrigerationSystem> systems() const;
+  // Returns the parent RefrigerationSystem if any
+  boost::optional<RefrigerationSystem> system() const;
 
   //@}
   /** @name Setters */
@@ -132,8 +132,8 @@ class MODEL_API RefrigerationCompressor : public ParentObject {
 
   void resetTranscriticalCompressorCapacityCurve();
 
-  // Remove from parent system(s)
-  void removeFromSystems();
+  // Remove from parentt system if any
+  void removeFromSystem();
 
   //@}
   /** @name Other */

src/model/RefrigerationCompressor_Impl.hpp

@@ -105,11 +105,7 @@ namespace detail {
 
     boost::optional<CurveBicubic> transcriticalCompressorCapacityCurve() const;
 
-    // There isn't any unicity enforced, so a compressor can be listed multiple times on a single system, or on several system
-    // Additionally, a compressor can be either listed on the CompressorList or the High Stage Compressor List
-    // Returns systems this is listed on, either as compressor or high stage compressor
-    std::vector<RefrigerationSystem> systems() const;
-
+    boost::optional<RefrigerationSystem> system() const;
 
     //@}
     /** @name Setters */
@@ -151,7 +147,7 @@ namespace detail {
 
     void resetTranscriticalCompressorCapacityCurve();
 
-    void removeFromSystems();
+    void removeFromSystem();
 
     //@}
     /** @name Other */

src/model/RefrigerationCondenserAirCooled.cpp

@@ -30,6 +30,8 @@
 #include "RefrigerationCondenserAirCooled.hpp"
 #include "RefrigerationCondenserAirCooled_Impl.hpp"
 
+#include "RefrigerationSystem.hpp"
+#include "RefrigerationSystem_Impl.hpp"
 #include "CurveLinear.hpp"
 #include "CurveLinear_Impl.hpp"
 #include "ThermalZone.hpp"
@@ -336,6 +338,21 @@ namespace detail {
     OS_ASSERT(result);
   }
 
+  boost::optional<RefrigerationSystem> RefrigerationCondenserAirCooled_Impl::system() const {
+
+    boost::optional<RefrigerationSystem> system;
+    // We use getModelObjectSources to check if more than one
+    std::vector<RefrigerationSystem> systems = getObject<ModelObject>().getModelObjectSources<RefrigerationSystem>(RefrigerationSystem::iddObjectType());
+
+    if( systems.size() > 0u) {
+      if( systems.size() > 1u) {
+        LOG(Error, briefDescription() << " is referenced by more than one RefrigerationSystem, returning the first");
+      }
+      system = systems[0];
+    }
+    return system;
+  }
+
 } // detail
 
 RefrigerationCondenserAirCooled::RefrigerationCondenserAirCooled(const Model& model)
@@ -522,6 +539,10 @@ void RefrigerationCondenserAirCooled::resetCondensatePipingRefrigerantInventory(
   getImpl<detail::RefrigerationCondenserAirCooled_Impl>()->resetCondensatePipingRefrigerantInventory();
 }
 
+boost::optional<RefrigerationSystem> RefrigerationCondenserAirCooled::system() const {
+  return getImpl<detail::RefrigerationCondenserAirCooled_Impl>()->system();
+}
+
 /// @cond
 RefrigerationCondenserAirCooled::RefrigerationCondenserAirCooled(std::shared_ptr<detail::RefrigerationCondenserAirCooled_Impl> impl)
   : ParentObject(std::move(impl))

src/model/RefrigerationCondenserAirCooled.hpp

@@ -39,6 +39,7 @@ namespace model {
 
 class CurveLinear;
 class ThermalZone;
+class RefrigerationSystem;
 
 namespace detail {
 
@@ -149,6 +150,10 @@ class MODEL_API RefrigerationCondenserAirCooled : public ParentObject {
   /** @name Other */
   //@{
 
+  // The parent RefrigerationSystem, which rejects heat to this condenser
+  // This is a convenience method to find any RefrigerationSystem that uses this condenser
+  boost::optional<RefrigerationSystem> system() const;
+
   //@}
  protected:
   /// @cond

src/model/RefrigerationCondenserAirCooled_Impl.hpp

@@ -36,6 +36,7 @@
 namespace openstudio {
 namespace model {
 
+class RefrigerationSystem;
 class CurveLinear;
 class ThermalZone;
 
@@ -163,6 +164,8 @@ namespace detail {
     /** @name Other */
     //@{
 
+    boost::optional<RefrigerationSystem> system() const;
+
     //@}
    protected:
    private: