Skip to content

Fix GroundHeatExchangerXXX cloning to another model #5460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joseph-robertson wants to merge 9 commits into
base: develop
Choose a base branch
from
+97 −10
Draft

Fix GroundHeatExchangerXXX cloning to another model #5460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5a22da9
Add ground hx model tests for cloning into another model.
joseph-robertson Jul 21, 2025
5da32dc
Formatting.
joseph-robertson Jul 21, 2025
b5929e4
Typo in new tests.
joseph-robertson Jul 21, 2025
fcedbce
Update clone methods for ground hxs.
joseph-robertson Jul 21, 2025
2dc84a1
Include Model in GHE horizontal trench.
joseph-robertson Jul 21, 2025
796cbb8
Update existing GHE horizontal trench model test expected values.
joseph-robertson Jul 21, 2025
c7621c8
Add a children method with SGT element.
joseph-robertson Jul 22, 2025
aa141f0
Additional remove model tests for GHE horizontal trench.
joseph-robertson Jul 22, 2025
786f151
Still having trouble with removing sgt even when connected.
joseph-robertson Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/model/GroundHeatExchangerHorizontalTrench.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "PlantLoop_Impl.hpp"
#include "Node.hpp"
#include "Node_Impl.hpp"
#include "Model.hpp"
#include "Model_Impl.hpp"
#include "SiteGroundTemperatureUndisturbedKusudaAchenbach.hpp"
#include "SiteGroundTemperatureUndisturbedKusudaAchenbach_Impl.hpp"

Expand Down Expand Up @@ -259,6 +261,22 @@ namespace model {
return false;
}

ModelObject GroundHeatExchangerHorizontalTrench_Impl::clone(Model model) const {
auto groundHeatExchangerClone = StraightComponent_Impl::clone(model).cast<GroundHeatExchangerHorizontalTrench>();

groundHeatExchangerClone.setUndisturbedGroundTemperatureModel(undisturbedGroundTemperatureModel().clone(model));

return std::move(groundHeatExchangerClone);
}

std::vector<ModelObject> GroundHeatExchangerHorizontalTrench_Impl::children() const {
std::vector<ModelObject> result;

result.push_back(undisturbedGroundTemperatureModel());

return result;
}
Comment on lines +272 to +278
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It this new children method necessary? I'm still not really sure how this method gets used downstream.


ComponentType GroundHeatExchangerHorizontalTrench_Impl::componentType() const {
return ComponentType::Both;
}
Expand Down
4 changes: 4 additions & 0 deletions src/model/GroundHeatExchangerHorizontalTrench_Impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ namespace model {

virtual IddObjectType iddObjectType() const override;

virtual ModelObject clone(Model model) const override;

virtual std::vector<ModelObject> children() const override;

virtual unsigned inletPort() const override;

virtual unsigned outletPort() const override;
Expand Down
15 changes: 12 additions & 3 deletions src/model/GroundHeatExchangerVertical.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,20 @@ namespace model {
return false;
}

//clone object
ModelObject GroundHeatExchangerVertical_Impl::clone(Model model) const {
auto newGroundHeatExchanger = StraightComponent_Impl::clone(model).cast<GroundHeatExchangerVertical>();
auto groundHeatExchangerClone = StraightComponent_Impl::clone(model).cast<GroundHeatExchangerVertical>();

return std::move(newGroundHeatExchanger);
groundHeatExchangerClone.setUndisturbedGroundTemperatureModel(undisturbedGroundTemperatureModel().clone(model));

return std::move(groundHeatExchangerClone);
}

std::vector<ModelObject> GroundHeatExchangerVertical_Impl::children() const {
std::vector<ModelObject> result;

result.push_back(undisturbedGroundTemperatureModel());

return result;
}

bool GroundHeatExchangerVertical_Impl::addGFunction(double gFunctionLN, double gFunctionGValue) {
Expand Down
2 changes: 2 additions & 0 deletions src/model/GroundHeatExchangerVertical_Impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ namespace model {

virtual ModelObject clone(Model model) const override;

virtual std::vector<ModelObject> children() const override;

virtual unsigned inletPort() const override;

virtual unsigned outletPort() const override;
Expand Down
56 changes: 49 additions & 7 deletions src/model/test/GroundHeatExchangerHorizontalTrench_GTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ TEST_F(ModelFixture, GroundHeatExchangerHorizontalTrench_Clone) {

auto ghClone = gh.clone(m).cast<GroundHeatExchangerHorizontalTrench>();
EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());
EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should clone into the same model create a new SGT object here? If not, how can this be prevented?

Copy link
Collaborator Author

@joseph-robertson joseph-robertson Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a precedent for determining whether to clone based on the model that the GHE is in. And I don't see an issue with just cloning into the same model.


ModelObject undisturbedGroundTemperatureModel = gh.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> uka =
Expand All @@ -201,15 +201,23 @@ TEST_F(ModelFixture, GroundHeatExchangerHorizontalTrench_Clone) {
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> ukaClone =
undisturbedGroundTemperatureModelClone.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(ukaClone);
EXPECT_EQ(sgt, ukaClone.get());
EXPECT_NE(sgt, ukaClone.get());

gh.remove();
EXPECT_EQ(1u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());

GroundHeatExchangerHorizontalTrench gh2(m, ukaClone.get());
EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
ghClone.remove();
EXPECT_EQ(0u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());

ModelObject undisturbedGroundTemperatureModel2 = gh2.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> uka2 =
undisturbedGroundTemperatureModel2.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(uka2);
EXPECT_EQ(ukaClone.get(), uka2.get());
}

{
Expand All @@ -222,7 +230,7 @@ TEST_F(ModelFixture, GroundHeatExchangerHorizontalTrench_Clone) {

auto ghClone = gh.clone(m).cast<GroundHeatExchangerHorizontalTrench>();
EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedXing>().size());
EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedXing>().size());

ModelObject undisturbedGroundTemperatureModel = gh.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedXing> ux = undisturbedGroundTemperatureModel.optionalCast<SiteGroundTemperatureUndisturbedXing>();
Expand All @@ -231,16 +239,50 @@ TEST_F(ModelFixture, GroundHeatExchangerHorizontalTrench_Clone) {

ModelObject undisturbedGroundTemperatureModelClone = ghClone.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedXing> uxClone =
undisturbedGroundTemperatureModel.optionalCast<SiteGroundTemperatureUndisturbedXing>();
undisturbedGroundTemperatureModelClone.optionalCast<SiteGroundTemperatureUndisturbedXing>();
ASSERT_TRUE(uxClone);
EXPECT_EQ(sgt, uxClone.get());
EXPECT_NE(sgt, uxClone.get());

gh.remove();
EXPECT_EQ(1u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedXing>().size());

GroundHeatExchangerHorizontalTrench gh2(m, uxClone.get());
EXPECT_EQ(2u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
ghClone.remove();
EXPECT_EQ(0u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<GroundHeatExchangerHorizontalTrench>().size());
EXPECT_EQ(1u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedXing>().size());

ModelObject undisturbedGroundTemperatureModel2 = gh2.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedXing> ux2 =
undisturbedGroundTemperatureModel2.optionalCast<SiteGroundTemperatureUndisturbedXing>();
ASSERT_TRUE(ux2);
EXPECT_EQ(uxClone.get(), ux2.get());
}

//clone into another model
{
Model m;
GroundHeatExchangerHorizontalTrench testObject(m);

auto testObjectClone = testObject.clone(m).cast<GroundHeatExchangerHorizontalTrench>();

Model m2;
auto testObjectClone2 = testObject.clone(m2).cast<GroundHeatExchangerHorizontalTrench>();

EXPECT_NE(testObjectClone2, testObjectClone);
EXPECT_NE(testObjectClone2.handle(), testObjectClone.handle());

ModelObject undisturbedGroundTemperatureModelClone = testObjectClone.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> sgtClone =
undisturbedGroundTemperatureModelClone.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(sgtClone);
EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());

ModelObject undisturbedGroundTemperatureModelClone2 = testObjectClone2.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> sgtClone2 =
undisturbedGroundTemperatureModelClone2.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(sgtClone2);
EXPECT_EQ(1u, m2.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());
}
}
12 changes: 12 additions & 0 deletions src/model/test/GroundHeatExchangerVertical_GTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,18 @@ TEST_F(ModelFixture, GroundHeatExchangerVertical_Clone) {

EXPECT_NE(testObjectClone2, testObjectClone);
EXPECT_NE(testObjectClone2.handle(), testObjectClone.handle());

ModelObject undisturbedGroundTemperatureModelClone = testObjectClone.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> sgtClone =
undisturbedGroundTemperatureModelClone.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(sgtClone);
EXPECT_EQ(2u, m.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());

ModelObject undisturbedGroundTemperatureModelClone2 = testObjectClone2.undisturbedGroundTemperatureModel();
boost::optional<SiteGroundTemperatureUndisturbedKusudaAchenbach> sgtClone2 =
undisturbedGroundTemperatureModelClone2.optionalCast<SiteGroundTemperatureUndisturbedKusudaAchenbach>();
ASSERT_TRUE(sgtClone2);
EXPECT_EQ(1u, m2.getConcreteModelObjects<SiteGroundTemperatureUndisturbedKusudaAchenbach>().size());
}

TEST_F(ModelFixture, GroundHeatExchangerVertical_GFunctions) {
Expand Down
Loading