Repository: NREL/OpenStudio (language: python)

Issue:
Cloning an object that uses a TableLookup into another model doesn't bring the ModelObjectList and IndependentVariables
Issue overview
--------------

If I clone a TableLookup into another model, all is well. If I clone a ModelObject that uses a TableLookup into another model it goes sideways, the  ModelObjectList that serves as the Table:IndependentVariableList isn't clone and neither are the TableIndependentVariable objects

## Current Behavior

See above

## Expected Behavior

The object should be valid

## Steps to Reproduce

```ruby
include OpenStudio::Model

m = Model.new

cc = CoilCoolingFourPipeBeam.new(m)
m.save('ori.osm', true)

m2 = Model.new
cc.clone(m2)
m2.save('new.osm', true)
```

<img width="1194" alt="image" src="https://user-images.githubusercontent.com/5479063/204808026-d126c588-bf60-471f-bdc0-e05964f45909.png">


## Possible Solution

The children/resources hierarchy is probably broken somewhere.

## Details

### Environment

Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version): All
 - Version of OpenStudio (if using an intermediate build, include SHA): 3.5.0

### Context

cf https://github.com/NREL/openstudio-aedg-gem/issues/30

Cloning an object that uses a TableLookup into another model doesn't bring the ModelObjectList and IndependentVariables
Issue overview
--------------

If I clone a TableLookup into another model, all is well. If I clone a ModelObject that uses a TableLookup into another model it goes sideways, the  ModelObjectList that serves as the Table:IndependentVariableList isn't clone and neither are the TableIndependentVariable objects

## Current Behavior

See above

## Expected Behavior

The object should be valid

## Steps to Reproduce

```ruby
include OpenStudio::Model

m = Model.new

cc = CoilCoolingFourPipeBeam.new(m)
m.save('ori.osm', true)

m2 = Model.new
cc.clone(m2)
m2.save('new.osm', true)
```

<img width="1194" alt="image" src="https://user-images.githubusercontent.com/5479063/204808026-d126c588-bf60-471f-bdc0-e05964f45909.png">


## Possible Solution

The children/resources hierarchy is probably broken somewhere.

## Details

### Environment

Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version): All
 - Version of OpenStudio (if using an intermediate build, include SHA): 3.5.0

### Context

cf https://github.com/NREL/openstudio-aedg-gem/issues/30

Cloning an object that uses a TableLookup into another model doesn't bring the ModelObjectList and IndependentVariables
Issue overview
--------------

If I clone a TableLookup into another model, all is well. If I clone a ModelObject that uses a TableLookup into another model it goes sideways, the  ModelObjectList that serves as the Table:IndependentVariableList isn't clone and neither are the TableIndependentVariable objects

## Current Behavior

See above

## Expected Behavior

The object should be valid

## Steps to Reproduce

```ruby
include OpenStudio::Model

m = Model.new

cc = CoilCoolingFourPipeBeam.new(m)
m.save('ori.osm', true)

m2 = Model.new
cc.clone(m2)
m2.save('new.osm', true)
```

<img width="1194" alt="image" src="https://user-images.githubusercontent.com/5479063/204808026-d126c588-bf60-471f-bdc0-e05964f45909.png">


## Possible Solution

The children/resources hierarchy is probably broken somewhere.

## Details

### Environment

Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version): All
 - Version of OpenStudio (if using an intermediate build, include SHA): 3.5.0

### Context

cf https://github.com/NREL/openstudio-aedg-gem/issues/30



Discussion / hints:

Fix patch:
diff --git a/src/model/TableLookup.cpp b/src/model/TableLookup.cpp
index df95a933ed3..f525f12c598 100644
--- a/src/model/TableLookup.cpp
+++ b/src/model/TableLookup.cpp
@@ -91,6 +91,17 @@ namespace model {
       return -9999.0;
     }
 
+    std::vector<ModelObject> TableLookup_Impl::children() const {
+      // #4754 - in the case where we clone an object that uses this TableLookup as a resource into another model,
+      // we need the ModelObjectList to be a child of this object, so that ResourceObject::getRecursiveResourceSubTrees finds it
+      // (and the variables it references in a recursive manner)
+      std::vector<ModelObject> result;
+      if (auto const varList_ = independentVariableList()) {
+        result.push_back(varList_.get());
+      }
+      return result;
+    }
+
     ModelObject TableLookup_Impl::clone(Model model) const {
 
       // Call the ModelObject_Impl instead of ParentObject_Impl because we deal with the ModelObjectList ourselves
diff --git a/src/model/TableLookup_Impl.hpp b/src/model/TableLookup_Impl.hpp
index f04d2f8a09c..79e4c3b5eef 100644
--- a/src/model/TableLookup_Impl.hpp
+++ b/src/model/TableLookup_Impl.hpp
@@ -70,6 +70,8 @@ namespace model {
 
       virtual ModelObject clone(Model model) const override;
 
+      virtual std::vector<ModelObject> children() const override;
+
       virtual std::vector<openstudio::IdfObject> remove() override;
 
       //@}


Test patch:
diff --git a/src/model/test/TableLookup_GTest.cpp b/src/model/test/TableLookup_GTest.cpp
index 68d84ced1a6..717c9e0568e 100644
--- a/src/model/test/TableLookup_GTest.cpp
+++ b/src/model/test/TableLookup_GTest.cpp
@@ -37,6 +37,8 @@
 #include "../TableIndependentVariable_Impl.hpp"
 #include "../ModelObjectList.hpp"
 #include "../ModelObjectList_Impl.hpp"
+#include "../BoilerHotWater.hpp"
+#include "../BoilerHotWater_Impl.hpp"
 
 using namespace openstudio;
 using namespace openstudio::model;
@@ -152,22 +154,22 @@ TEST_F(ModelFixture, TableLookup_Clone) {
   TableIndependentVariable independentVariable1(m);
   EXPECT_TRUE(tableLookup.addIndependentVariable(independentVariable1));
 
-  EXPECT_EQ(1u, m.getModelObjects<TableLookup>().size());
-  EXPECT_EQ(1u, m.getModelObjects<ModelObjectList>().size());
-  EXPECT_EQ(1u, m.getModelObjects<TableIndependentVariable>().size());
-  EXPECT_EQ(1u, tableLookup.independentVariables().size());
+  EXPECT_EQ(1, m.getModelObjects<TableLookup>().size());
+  EXPECT_EQ(1, m.getModelObjects<ModelObjectList>().size());
+  EXPECT_EQ(1, m.getModelObjects<TableIndependentVariable>().size());
+  EXPECT_EQ(1, tableLookup.independentVariables().size());
 
   // Clone into the same model
   {
     auto tableLookupClone = tableLookup.clone(m).cast<TableLookup>();
-    EXPECT_EQ(2u, m.getModelObjects<TableLookup>().size());
-    EXPECT_EQ(2u, m.getModelObjects<ModelObjectList>().size());
-    EXPECT_EQ(1u, m.getModelObjects<TableIndependentVariable>().size());
+    EXPECT_EQ(2, m.getModelObjects<TableLookup>().size());
+    EXPECT_EQ(2, m.getModelObjects<ModelObjectList>().size());
+    EXPECT_EQ(1, m.getModelObjects<TableIndependentVariable>().size());
 
-    ASSERT_EQ(1u, tableLookup.independentVariables().size());
+    ASSERT_EQ(1, tableLookup.independentVariables().size());
     EXPECT_EQ(independentVariable1, tableLookup.independentVariables().front());
 
-    ASSERT_EQ(1u, tableLookupClone.independentVariables().size());
+    ASSERT_EQ(1, tableLookupClone.independentVariables().size());
     EXPECT_EQ(independentVariable1, tableLookupClone.independentVariables().front());
   }
 
@@ -177,19 +179,57 @@ TEST_F(ModelFixture, TableLookup_Clone) {
     auto tableLookupClone2 = tableLookup.clone(m2).cast<TableLookup>();
 
     // Check original model is unaffected
-    EXPECT_EQ(2u, m.getModelObjects<TableLookup>().size());
-    EXPECT_EQ(2u, m.getModelObjects<ModelObjectList>().size());
-    EXPECT_EQ(1u, m.getModelObjects<TableIndependentVariable>().size());
+    EXPECT_EQ(2, m.getModelObjects<TableLookup>().size());
+    EXPECT_EQ(2, m.getModelObjects<ModelObjectList>().size());
+    EXPECT_EQ(1, m.getModelObjects<TableIndependentVariable>().size());
 
-    ASSERT_EQ(1u, tableLookup.independentVariables().size());
+    ASSERT_EQ(1, tableLookup.independentVariables().size());
     EXPECT_EQ(independentVariable1, tableLookup.independentVariables().front());
 
     // Check that the TableLookup and TableLookupIndependentVariable are carried into new model
-    EXPECT_EQ(1u, m2.getModelObjects<TableLookup>().size());
-    EXPECT_EQ(1u, m2.getModelObjects<ModelObjectList>().size());
-    EXPECT_EQ(1u, m2.getModelObjects<TableIndependentVariable>().size());
+    EXPECT_EQ(1, m2.getModelObjects<TableLookup>().size());
+    EXPECT_EQ(1, m2.getModelObjects<ModelObjectList>().size());
+    EXPECT_EQ(1, m2.getModelObjects<TableIndependentVariable>().size());
 
-    ASSERT_EQ(1u, tableLookupClone2.independentVariables().size());
+    ASSERT_EQ(1, tableLookupClone2.independentVariables().size());
+    EXPECT_NE(independentVariable1, tableLookupClone2.independentVariables().front());
+  }
+
+  BoilerHotWater boiler(m);
+  EXPECT_TRUE(boiler.setNormalizedBoilerEfficiencyCurve(tableLookup));
+  EXPECT_EQ(6, m.numObjects());
+  EXPECT_EQ(2, m.getModelObjects<TableLookup>().size());
+  EXPECT_EQ(2, m.getModelObjects<ModelObjectList>().size());
+  EXPECT_EQ(1, m.getModelObjects<TableIndependentVariable>().size());
+  EXPECT_EQ(1, m.getModelObjects<BoilerHotWater>().size());
+
+  EXPECT_EQ(1, tableLookup.independentVariables().size());
+
+  // Clone a component that references a TableLookup into another model
+  {
+    Model m2;
+    auto boilerClone = boiler.clone(m2).cast<BoilerHotWater>();
+
+    ASSERT_TRUE(boilerClone.normalizedBoilerEfficiencyCurve());
+    auto tableLookupClone2 = boilerClone.normalizedBoilerEfficiencyCurve()->cast<TableLookup>();
+
+    // Check original model is unaffected
+    EXPECT_EQ(6, m.numObjects());
+    EXPECT_EQ(2, m.getModelObjects<TableLookup>().size());
+    EXPECT_EQ(2, m.getModelObjects<ModelObjectList>().size());
+    EXPECT_EQ(1, m.getModelObjects<TableIndependentVariable>().size());
+    EXPECT_EQ(1, m.getModelObjects<BoilerHotWater>().size());
+
+    EXPECT_EQ(1, tableLookup.independentVariables().size());
+
+    // Check that the TableLookup and TableLookupIndependentVariable are carried into the new model
+    EXPECT_EQ(4, m2.numObjects());
+    EXPECT_EQ(1, m2.getModelObjects<TableLookup>().size());
+    EXPECT_EQ(1, m2.getModelObjects<ModelObjectList>().size());
+    EXPECT_EQ(1, m2.getModelObjects<TableIndependentVariable>().size());
+    EXPECT_EQ(1, m2.getModelObjects<BoilerHotWater>().size());
+
+    ASSERT_EQ(1, tableLookupClone2.independentVariables().size());
     EXPECT_NE(independentVariable1, tableLookupClone2.independentVariables().front());
   }
 }