Quick fix #4716 by jmarrec · Pull Request #4733 · NatLabRockies/OpenStudio

jmarrec · 2022-10-31T10:41:09Z

Pull request overview

Minimal Fix GroundHeatExchangerHorizontalTrench : incomplete IDF translation #4716

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: Add Link
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

jmarrec · 2022-10-31T11:46:19Z

So before fix tests logically fails: https://ci.openstudio.net/blue/organizations/jenkins/openstudio-incremental-ubuntu-18-04/detail/PR-4733/2/pipeline/68#step-72-log-19

jmarrec · 2022-10-31T11:47:08Z

src/energyplus/ForwardTranslator/ForwardTranslateGroundHeatExchangerHorizontalTrench.cpp

+      idfObject.setString(GroundHeatExchanger_HorizontalTrenchFields::UndisturbedGroundTemperatureModelType,
+                          "Site:GroundTemperature:Undisturbed:KusudaAchenbach");


Write model type

jmarrec · 2022-10-31T11:47:19Z

src/energyplus/ForwardTranslator/ForwardTranslateGroundHeatExchangerHorizontalTrench.cpp

+      // SoilThermalConductivity
+      {
+        auto value = modelObject.soilThermalConductivity();
+        groundModel.setDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilThermalConductivity, value);
+      }
+
+      // SoilDensity
+      {
+        auto value = modelObject.soilDensity();
+        groundModel.setDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilDensity, value);
+      }
+
+      // SoilSpecificHeat
+      {
+        auto value = modelObject.soilSpecificHeat();
+        groundModel.setDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilSpecificHeat, value);
+      }


Write soil properties on the Kusuda object

jmarrec · 2022-10-31T11:48:11Z

@tijcolem @joseph-robertson I recommend pulling this into the 3.5.0 branch, this is safe. Only fixes the FT, doesn't affect model. PLus I added tests

jmarrec · 2022-10-31T14:06:05Z

src/energyplus/Test/GroundHeatExchangerHorizontalTrench_GTest.cpp

+TEST_F(EnergyPlusFixture, ForwardTranslator_GroundHeatExchangerHorizontalTrench) {
+
+  ForwardTranslator ft;
+
+  // Create a model
+  Model m;
+
+  GroundHeatExchangerHorizontalTrench ghx(m);
+
+  EXPECT_TRUE(ghx.setDesignFlowRate(0.05));
+  EXPECT_TRUE(ghx.setTrenchLengthinPipeAxialDirection(72.0));
+  EXPECT_TRUE(ghx.setNumberofTrenches(3));
+  EXPECT_TRUE(ghx.setHorizontalSpacingBetweenPipes(2.1));
+  EXPECT_TRUE(ghx.setPipeInnerDiameter(0.019));
+  EXPECT_TRUE(ghx.setPipeOuterDiameter(0.029));
+  EXPECT_TRUE(ghx.setBurialDepth(1.35));
+  EXPECT_TRUE(ghx.setSoilThermalConductivity(1.02));
+  EXPECT_TRUE(ghx.setSoilDensity(970.0));
+  EXPECT_TRUE(ghx.setSoilSpecificHeat(2430.0));
+  EXPECT_TRUE(ghx.setPipeThermalConductivity(0.32));
+  EXPECT_TRUE(ghx.setPipeDensity(620.0));
+  EXPECT_TRUE(ghx.setPipeSpecificHeat(2200.0));
+  EXPECT_TRUE(ghx.setSoilMoistureContentPercent(32.0));
+  EXPECT_TRUE(ghx.setSoilMoistureContentPercentatSaturation(53.0));
+  EXPECT_TRUE(ghx.setGroundTemperatureModel("KusudaAchenbach"));
+  EXPECT_TRUE(ghx.setKusudaAchenbachAverageSurfaceTemperature(16.5));
+  EXPECT_TRUE(ghx.setKusudaAchenbachAverageAmplitudeofSurfaceTemperature(13.8));
+  EXPECT_TRUE(ghx.setKusudaAchenbachPhaseShiftofMinimumSurfaceTemperature(18.3));
+  EXPECT_TRUE(ghx.setEvapotranspirationGroundCoverParameter(0.358));
+
+  PlantLoop p(m);
+  EXPECT_TRUE(p.addSupplyBranchForComponent(ghx));
+
+  ASSERT_TRUE(ghx.inletModelObject());
+  ASSERT_TRUE(ghx.inletModelObject()->optionalCast<Node>());
+  ghx.inletModelObject()->cast<Node>().setName("GHX Inlet Node");
+
+  ASSERT_TRUE(ghx.outletModelObject());
+  ASSERT_TRUE(ghx.outletModelObject()->optionalCast<Node>());
+  ghx.outletModelObject()->cast<Node>().setName("GHX Outlet Node");
+
+  const Workspace w = ft.translateModel(m);
+
+  WorkspaceObjectVector idfObjs = w.getObjectsByType(IddObjectType::GroundHeatExchanger_HorizontalTrench);
+  ASSERT_EQ(1u, idfObjs.size());
+  auto idfObject = idfObjs.front();
+
+  EXPECT_EQ("Ground Heat Exchanger Horizontal Trench 1", idfObject.getString(GroundHeatExchanger_HorizontalTrenchFields::Name).get());
+  EXPECT_EQ("GHX Inlet Node", idfObject.getString(GroundHeatExchanger_HorizontalTrenchFields::InletNodeName).get());
+  EXPECT_EQ("GHX Outlet Node", idfObject.getString(GroundHeatExchanger_HorizontalTrenchFields::OutletNodeName).get());
+  EXPECT_EQ(0.05, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::DesignFlowRate).get());
+  EXPECT_EQ(72.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::TrenchLengthinPipeAxialDirection).get());
+  EXPECT_EQ(3, idfObject.getInt(GroundHeatExchanger_HorizontalTrenchFields::NumberofTrenches).get());
+  EXPECT_EQ(2.1, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::HorizontalSpacingBetweenPipes).get());
+  EXPECT_EQ(0.019, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::PipeInnerDiameter).get());
+  EXPECT_EQ(0.029, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::PipeOuterDiameter).get());
+  EXPECT_EQ(1.35, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::BurialDepth).get());
+  EXPECT_EQ(1.02, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::SoilThermalConductivity).get());
+  EXPECT_EQ(970.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::SoilDensity).get());
+  EXPECT_EQ(2430.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::SoilSpecificHeat).get());
+  EXPECT_EQ(0.32, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::PipeThermalConductivity).get());
+  EXPECT_EQ(620.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::PipeDensity).get());
+  EXPECT_EQ(2200.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::PipeSpecificHeat).get());
+  EXPECT_EQ(32.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::SoilMoistureContentPercent).get());
+  EXPECT_EQ(53.0, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::SoilMoistureContentPercentatSaturation).get());
+
+  EXPECT_EQ("Site:GroundTemperature:Undisturbed:KusudaAchenbach",
+            idfObject.getString(GroundHeatExchanger_HorizontalTrenchFields::UndisturbedGroundTemperatureModelType).get());
+
+  ASSERT_TRUE(idfObject.getTarget(GroundHeatExchanger_HorizontalTrenchFields::UndisturbedGroundTemperatureModelName));
+  const WorkspaceObject kusuda = idfObject.getTarget(GroundHeatExchanger_HorizontalTrenchFields::UndisturbedGroundTemperatureModelName).get();
+  EXPECT_EQ(IddObjectType{IddObjectType::Site_GroundTemperature_Undisturbed_KusudaAchenbach}, kusuda.iddObject().type());
+  EXPECT_EQ(0.358, idfObject.getDouble(GroundHeatExchanger_HorizontalTrenchFields::EvapotranspirationGroundCoverParameter).get());
+
+  EXPECT_EQ(1.02, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilThermalConductivity).get());
+  EXPECT_EQ(970.0, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilDensity).get());
+  EXPECT_EQ(2430.0, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::SoilSpecificHeat).get());
+  EXPECT_EQ(16.5, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::AverageSoilSurfaceTemperature).get());
+  EXPECT_EQ(13.8, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::AverageAmplitudeofSurfaceTemperature).get());
+  EXPECT_EQ(18.3, kusuda.getDouble(Site_GroundTemperature_Undisturbed_KusudaAchenbachFields::PhaseShiftofMinimumSurfaceTemperature).get());


New full FT Gtest

ci-commercialbuildings · 2022-10-31T14:15:29Z

CI Results for 2b8e677:

Ubuntu 18.04:
- OpenStudio-3.5.0-rc1+2b8e67738c-Ubuntu-18.04.deb
- OpenStudio-3.5.0-rc1+2b8e67738c-Ubuntu-18.04.tar.gz
Ubuntu 20.04:
Darwin:
- OpenStudio-3.5.0-rc1+2b8e67738c-Darwin-x86_64.dmg
- OpenStudio-3.5.0-rc1+2b8e67738c-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.5.0-rc1+2b8e67738c-Windows.exe
- OpenStudio-3.5.0-rc1+2b8e67738c-Windows.tar.gz
Code Coverage:

tijcolem

Looks good to me. The BCL tests are failing which is unrelate to this PR.

jmarrec added severity - Minor Bug component - HVAC component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Oct 31, 2022

jmarrec requested a review from joseph-robertson October 31, 2022 10:41

jmarrec self-assigned this Oct 31, 2022

jmarrec mentioned this pull request Oct 31, 2022

GroundHeatExchangerHorizontalTrench : incomplete IDF translation #4716

Closed

jmarrec force-pushed the 4716_quickfix branch from 3d037a0 to 8cae6f8 Compare October 31, 2022 11:34

NatLabRockies deleted a comment from ci-commercialbuildings Oct 31, 2022

jmarrec commented Oct 31, 2022

View reviewed changes

jmarrec requested a review from tijcolem October 31, 2022 11:47

jmarrec added this to the OpenStudio SDK 3.5.0 milestone Oct 31, 2022

jmarrec added 2 commits October 31, 2022 15:05

Add a Gtest for GroundHeatExchangerHorizontalTrench (#4716)

aaaea48

Quick fix #4716

2b8e677

jmarrec force-pushed the 4716_quickfix branch from 26a4a68 to 2b8e677 Compare October 31, 2022 14:05

jmarrec commented Oct 31, 2022

View reviewed changes

tijcolem approved these changes Oct 31, 2022

View reviewed changes

tijcolem merged commit 65bcf10 into develop Oct 31, 2022

tijcolem deleted the 4716_quickfix branch October 31, 2022 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quick fix #4716#4733

Quick fix #4716#4733
tijcolem merged 2 commits intodevelopfrom
4716_quickfix

jmarrec commented Oct 31, 2022

Uh oh!

jmarrec commented Oct 31, 2022

Uh oh!

jmarrec Oct 31, 2022

Uh oh!

jmarrec Oct 31, 2022

Uh oh!

jmarrec commented Oct 31, 2022

Uh oh!

jmarrec Oct 31, 2022

Uh oh!

ci-commercialbuildings commented Oct 31, 2022 •

edited

Loading

Uh oh!

tijcolem left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		idfObject.setString(GroundHeatExchanger_HorizontalTrenchFields::UndisturbedGroundTemperatureModelType,
		"Site:GroundTemperature:Undisturbed:KusudaAchenbach");

Conversation

jmarrec commented Oct 31, 2022

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

jmarrec commented Oct 31, 2022

Uh oh!

jmarrec Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

jmarrec Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Oct 31, 2022

Uh oh!

jmarrec Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

ci-commercialbuildings commented Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tijcolem left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ci-commercialbuildings commented Oct 31, 2022 •

edited

Loading