Skip to content
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.

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

Parse Ground Temperatures in EpwFile #5318

Merged
merged 6 commits into from
Dec 19, 2024
Merged

Parse Ground Temperatures in EpwFile #5318

merged 6 commits into from
Dec 19, 2024

Conversation

joseph-robertson
Copy link
Collaborator

@joseph-robertson joseph-robertson commented Dec 17, 2024

Pull request overview

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

@joseph-robertson joseph-robertson added Enhancement Request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Dec 17, 2024
@joseph-robertson joseph-robertson self-assigned this Dec 17, 2024
src/utilities/filetypes/EpwFile.cpp Outdated Show resolved Hide resolved
src/utilities/filetypes/EpwFile.cpp Outdated Show resolved Hide resolved
src/utilities/filetypes/EpwFile.cpp Outdated Show resolved Hide resolved
src/utilities/filetypes/EpwFile.cpp Show resolved Hide resolved
src/utilities/filetypes/EpwFile.hpp Outdated Show resolved Hide resolved
@jmarrec jmarrec marked this pull request as ready for review December 18, 2024 10:46
@jmarrec
Copy link
Collaborator

jmarrec commented Dec 18, 2024

@joseph-robertson I fixed up the implementation.

I was debating changing the Soil properties to be boost::optional, but after making the change I don't think it's really that much better than leaving a "-9999" value.

Click to see diff patch
diff --git a/src/utilities/filetypes/EpwFile.cpp b/src/utilities/filetypes/EpwFile.cpp
index 2efc50e6f4..6d6f31e85a 100644
--- a/src/utilities/filetypes/EpwFile.cpp
+++ b/src/utilities/filetypes/EpwFile.cpp
@@ -3635,16 +3635,14 @@ std::vector<EpwDesignCondition> EpwFile::designConditions() {
 *****************************************************************************************************************************************************/
 static constexpr size_t NUMBER_GROUND_TEMP_STRINGS = 16;
 
-EpwGroundTemperatureDepth::EpwGroundTemperatureDepth(double groundTemperatureDepth, double soilConductivity, double soilDensity,
-                                                     double soilSpecificHeat, double janGroundTemperature, double febGroundTemperature,
+EpwGroundTemperatureDepth::EpwGroundTemperatureDepth(double groundTemperatureDepth, double janGroundTemperature, double febGroundTemperature,
                                                      double marGroundTemperature, double aprGroundTemperature, double mayGroundTemperature,
                                                      double junGroundTemperature, double julGroundTemperature, double augGroundTemperature,
                                                      double sepGroundTemperature, double octGroundTemperature, double novGroundTemperature,
-                                                     double decGroundTemperature) {
+                                                     double decGroundTemperature, boost::optional<double> soilConductivity_,
+                                                     boost::optional<double> soilDensity_, boost::optional<double> soilSpecificHeat_) {
   setGroundTemperatureDepth(groundTemperatureDepth);
-  setSoilConductivity(soilConductivity);
-  setSoilDensity(soilDensity);
-  setSoilSpecificHeat(soilSpecificHeat);
+
   setJanGroundTemperature(janGroundTemperature);
   setFebGroundTemperature(febGroundTemperature);
   setMarGroundTemperature(marGroundTemperature);
@@ -3657,6 +3655,16 @@ EpwGroundTemperatureDepth::EpwGroundTemperatureDepth(double groundTemperatureDep
   setOctGroundTemperature(octGroundTemperature);
   setNovGroundTemperature(novGroundTemperature);
   setDecGroundTemperature(decGroundTemperature);
+
+  if (soilConductivity_) {
+    setSoilConductivity(*soilConductivity_);
+  }
+  if (soilDensity_) {
+    setSoilDensity(*soilDensity_);
+  }
+  if (soilSpecificHeat_) {
+    setSoilSpecificHeat(*soilSpecificHeat_);
+  }
 }
 
 boost::optional<EpwGroundTemperatureDepth> EpwGroundTemperatureDepth::fromGroundTemperatureDepthsString(const std::string& line) {
@@ -3832,15 +3840,15 @@ double EpwGroundTemperatureDepth::groundTemperatureDepth() const {
   return m_groundTemperatureDepth;
 }
 
-double EpwGroundTemperatureDepth::soilConductivity() const {
+boost::optional<double> EpwGroundTemperatureDepth::soilConductivity() const {
   return m_soilConductivity;
 }
 
-double EpwGroundTemperatureDepth::soilDensity() const {
+boost::optional<double> EpwGroundTemperatureDepth::soilDensity() const {
   return m_soilDensity;
 }
 
-double EpwGroundTemperatureDepth::soilSpecificHeat() const {
+boost::optional<double> EpwGroundTemperatureDepth::soilSpecificHeat() const {
   return m_soilSpecificHeat;
 }
 
diff --git a/src/utilities/filetypes/EpwFile.hpp b/src/utilities/filetypes/EpwFile.hpp
index d3f4174e84..82eae08b45 100644
--- a/src/utilities/filetypes/EpwFile.hpp
+++ b/src/utilities/filetypes/EpwFile.hpp
@@ -871,10 +871,13 @@ class UTILITIES_API EpwGroundTemperatureDepth
   /** Create an empty EpwGroundTemperatureDepth object */
   EpwGroundTemperatureDepth() = default;
   /** Create an EpwGroundTemperatureDepth object with specified properties */
-  EpwGroundTemperatureDepth(double groundTemperatureDepth, double soilConductivity, double soilDensity, double soilSpecificHeat,
-                            double janGroundTemperature, double febGroundTemperature, double marGroundTemperature, double aprGroundTemperature,
-                            double mayGroundTemperature, double junGroundTemperature, double julGroundTemperature, double augGroundTemperature,
-                            double sepGroundTemperature, double octGroundTemperature, double novGroundTemperature, double decGroundTemperature);
+  EpwGroundTemperatureDepth(double groundTemperatureDepth, double janGroundTemperature, double febGroundTemperature, double marGroundTemperature,
+                            double aprGroundTemperature, double mayGroundTemperature, double junGroundTemperature, double julGroundTemperature,
+                            double augGroundTemperature, double sepGroundTemperature, double octGroundTemperature, double novGroundTemperature,
+                            double decGroundTemperature,
+                            // Optional parameters
+                            boost::optional<double> soilConductivity_ = boost::none, boost::optional<double> soilDensity_ = boost::none,
+                            boost::optional<double> soilSpecificHeat_ = boost::none);
   // Static
   /** Returns the units of the named field */
   static boost::optional<std::string> getUnitsByName(const std::string& name);
@@ -894,11 +897,11 @@ class UTILITIES_API EpwGroundTemperatureDepth
   /** Returns the depth of ground temperature in m*/
   double groundTemperatureDepth() const;
   /** Returns the soil conductivity in W/m-K*/
-  double soilConductivity() const;
+  boost::optional<double> soilConductivity() const;
   /** Returns the soil density in kg/m3*/
-  double soilDensity() const;
+  boost::optional<double> soilDensity() const;
   /** Returns the soil specific heat in J/kg-K*/
-  double soilSpecificHeat() const;
+  boost::optional<double> soilSpecificHeat() const;
   /** Returns the Jan undisturbed ground temperature in degrees C*/
   double janGroundTemperature() const;
   /** Returns the Feb undisturbed ground temperature in degrees C*/
@@ -960,9 +963,9 @@ class UTILITIES_API EpwGroundTemperatureDepth
   bool setDecGroundTemperature(const std::string& decGroundTemperature);
 
   double m_groundTemperatureDepth = -9999;
-  double m_soilConductivity = -9999;
-  double m_soilDensity = -9999;
-  double m_soilSpecificHeat = -9999;
+  boost::optional<double> m_soilConductivity;
+  boost::optional<double> m_soilDensity;
+  boost::optional<double> m_soilSpecificHeat;
   double m_janGroundTemperature = -9999;
   double m_febGroundTemperature = -9999;
   double m_marGroundTemperature = -9999;
diff --git a/src/utilities/filetypes/test/EpwFile_GTest.cpp b/src/utilities/filetypes/test/EpwFile_GTest.cpp
index 78723af8de..b28dd62519 100644
--- a/src/utilities/filetypes/test/EpwFile_GTest.cpp
+++ b/src/utilities/filetypes/test/EpwFile_GTest.cpp
@@ -276,9 +276,24 @@ TEST(Filetypes, EpwFile_Ground) {
     const auto& expected_values = expected_depth_values.at(i);
 
     EXPECT_DOUBLE_EQ(expected_values.at(0), depth.groundTemperatureDepth());
-    EXPECT_DOUBLE_EQ(expected_values.at(1), depth.soilConductivity());
-    EXPECT_DOUBLE_EQ(expected_values.at(2), depth.soilDensity());
-    EXPECT_DOUBLE_EQ(expected_values.at(3), depth.soilSpecificHeat());
+    if (expected_values.at(1) < 0) {
+      EXPECT_FALSE(depth.soilConductivity());
+    } else {
+      ASSERT_TRUE(depth.soilConductivity());
+      EXPECT_DOUBLE_EQ(expected_values.at(1), depth.soilConductivity().get());
+    }
+    if (expected_values.at(2) < 0) {
+      EXPECT_FALSE(depth.soilDensity());
+    } else {
+      ASSERT_TRUE(depth.soilDensity());
+      EXPECT_DOUBLE_EQ(expected_values.at(2), depth.soilDensity().get());
+    }
+    if (expected_values.at(3) < 0) {
+      EXPECT_FALSE(depth.soilSpecificHeat());
+    } else {
+      ASSERT_TRUE(depth.soilSpecificHeat());
+      EXPECT_DOUBLE_EQ(expected_values.at(3), depth.soilSpecificHeat().get());
+    }
     EXPECT_DOUBLE_EQ(expected_values.at(4), depth.janGroundTemperature());
     EXPECT_DOUBLE_EQ(expected_values.at(5), depth.febGroundTemperature());
     EXPECT_DOUBLE_EQ(expected_values.at(6), depth.marGroundTemperature());

@joseph-robertson
Copy link
Collaborator Author

joseph-robertson commented Dec 18, 2024

@joseph-robertson I fixed up the implementation.

I was debating changing the Soil properties to be boost::optional, but after making the change I don't think it's really that much better than leaving a "-9999" value.

Click to see diff patch

Hm, maybe that's the approach (i.e., the "-9999") we should take for design conditions instead of #5134?

@jmarrec jmarrec merged commit 10eb60d into develop Dec 19, 2024
4 of 6 checks passed
@jmarrec jmarrec deleted the epw_ground_temps branch December 19, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse Ground Temperatures in EpwFile
3 participants