Repository: NREL/OpenStudio (language: python)

Issue:
gbxml export does not transfer thermalZone Volume
Issue overview
--------------
<!--- Provide a general summary of the issue -->

Space Volume calculations in openstudio are not accurate for complex shapes (https://github.com/NREL/OpenStudio/issues/127). As a workaround one can set osThermalZone.setVolume. While this fixes the issue in the .OSM file, the gbxml forward translator does not carry over that value and uses space.volume which can have wrong value. It should use space.thermalzone.volume instead. Or offer the use a method to set the volume for a space (osSpace.setVolume does NOT exist)

## Current Behavior
<!--- Tell us what happens instead of the expected behavior -->
For complex shapes, the volume of the space is wrong

## Expected Behavior
<!--- Tell us what should happen -->
One or all things should happen:
1. osSpace should calculate the volume in a more accurate fashion. Address https://github.com/NREL/OpenStudio/issues/127
2. osSpace should offer a osSpace.setVolume method
3. If the above is not fixed, The gbxml Forward translator should use the saved value in the thermalZone rather than in space.volume

## Steps to Reproduce
1. Open the attached SpeckleBuilding.zip file and extract the OSM and GBXML files
2. Using a robust geometric method, compute the volumes of Space "8" and Space "9"
3. Compare the Volumes of Space "8" and Space "9" in the gbxml file to your calculations
4. Export the OSM file to gbxml and observe the volume of Space "8" and Space "9" compare to the volume set for their thermal zones.

## Possible Solution
<!--- Optional, but if you can, suggest a fix/reason for the bug, -->
One or allthings should happen:
1. osSpace should calculate the volume in a more accurate fashion. Address https://github.com/NREL/OpenStudio/issues/127
2. osSpace should offer a osSpace.setVolume method
3. If the above is not fixed, The gbxml Forward translator should use the saved value in the thermalZone rather than in space.volume
## Details

### Environment
[SpeckleBuilding.zip](https://github.com/NREL/OpenStudio/files/8407059/SpeckleBuilding.zip)


Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version):
 - Version of OpenStudio (if using an intermediate build, include SHA): Application: 1.3.0  SDK: 3.3.0

### Context
<!--- How has this issue affected you? What are you trying to accomplish? -->
We are trying to output a robust and dependable gbxml from openstudio for energy analysis and a wrong volume can cause problems down the line.



Discussion / hints:
I think that points 1 and 2 should be addressed.  The OS volume calculations should match the E+ calculations and now that E+ has a space object I think it will make sense to be able to set the space volume directly if you know it more accurately than OS/E+.

On point 3, I think that would only be correct if the thermal zone has one space, otherwise it would be hard to assign the thermal zone volume among several spaces.
I could try to add a `Space::setVolume` method, #127 might be a good issue for @ggartside if he is looking for things to work on.
Hmm looks like the E+ space does not have a volume property so the OS space volume would not translate over to E+.  But OS space volume would translate to gbXML which is what @wassimj is looking at.
So our preference here would be to implement `Space::setVolume` and update `Space` IDD for a new `Volume` field? Then have `Space::volume` return the set value, otherwise the calculated value if not populated?

(If these are addressed, then there'd be no need for point 3 above.)

Fix patch:
diff --git a/resources/model/OpenStudio.idd b/resources/model/OpenStudio.idd
index d0f8ae6dd9d..d23cfc11d36 100644
--- a/resources/model/OpenStudio.idd
+++ b/resources/model/OpenStudio.idd
@@ -5298,9 +5298,14 @@ OS:Space,
   A9, \field Design Specification Outdoor Air Object Name
        \type object-list
        \object-list DesignSpecificationOutdoorAirNames
-  A10; \field Building Unit Name
+  A10, \field Building Unit Name
        \type object-list
        \object-list BuildingUnitNames
+  N5; \field Volume
+       \type real
+       \units m3
+       \autocalculatable
+       \default autocalculate
 
 OS:Surface,
        \extensible:3
diff --git a/src/model/Space.cpp b/src/model/Space.cpp
index 3eccbf7d6f0..853802f2865 100644
--- a/src/model/Space.cpp
+++ b/src/model/Space.cpp
@@ -496,6 +496,22 @@ namespace model {
       OS_ASSERT(result);
     }
 
+    bool Space_Impl::setVolume(double volume) {
+      bool result = setDouble(OS_SpaceFields::Volume, volume);
+      OS_ASSERT(result);
+      return result;
+    }
+
+    void Space_Impl::autocalculateVolume() {
+      bool result = setString(OS_SpaceFields::Volume, "Autocalculate");
+      OS_ASSERT(result);
+    }
+
+    void Space_Impl::resetVolume() {
+      bool result = setString(OS_SpaceFields::Volume, "");
+      OS_ASSERT(result);
+    }
+
     boost::optional<SpaceType> Space_Impl::spaceType() const {
       boost::optional<SpaceType> result = getObject<ModelObject>().getModelObjectTarget<SpaceType>(OS_SpaceFields::SpaceTypeName);
       if (!result) {
@@ -886,6 +902,11 @@ namespace model {
     }
 
     double Space_Impl::volume() const {
+      boost::optional<double> value = getDouble(OS_SpaceFields::Volume, true);
+      if (value) {
+        return value.get();
+      }
+
       double result = 0;
 
       // TODO: need a better method
@@ -916,6 +937,19 @@ namespace model {
       return result;
     }
 
+    bool Space_Impl::isVolumeDefaulted() const {
+      return isEmpty(OS_SpaceFields::Volume);
+    }
+
+    bool Space_Impl::isVolumeAutocalculated() const {
+      bool result = false;
+      boost::optional<std::string> value = getString(OS_SpaceFields::Volume, true);
+      if (value) {
+        result = openstudio::istringEqual(value.get(), "Autocalculate");
+      }
+      return result;
+    }
+
     double Space_Impl::numberOfPeople() const {
       double result = 0.0;
       double area = floorArea();
@@ -2863,6 +2897,18 @@ namespace model {
     getImpl<detail::Space_Impl>()->resetPartofTotalFloorArea();
   }
 
+  bool Space::setVolume(double volume) {
+    return getImpl<detail::Space_Impl>()->setVolume(volume);
+  }
+
+  void Space::autocalculateVolume() {
+    getImpl<detail::Space_Impl>()->autocalculateVolume();
+  }
+
+  void Space::resetVolume() {
+    getImpl<detail::Space_Impl>()->resetVolume();
+  }
+
   boost::optional<SpaceType> Space::spaceType() const {
     return getImpl<detail::Space_Impl>()->spaceType();
   }
@@ -3067,6 +3113,14 @@ namespace model {
     return getImpl<detail::Space_Impl>()->volume();
   }
 
+  bool Space::isVolumeDefaulted() const {
+    return getImpl<detail::Space_Impl>()->isVolumeDefaulted();
+  }
+
+  bool Space::isVolumeAutocalculated() const {
+    return getImpl<detail::Space_Impl>()->isVolumeAutocalculated();
+  }
+
   double Space::numberOfPeople() const {
     return getImpl<detail::Space_Impl>()->numberOfPeople();
   }
diff --git a/src/model/Space.hpp b/src/model/Space.hpp
index f3b27c8a1c6..9f024f5c7f5 100644
--- a/src/model/Space.hpp
+++ b/src/model/Space.hpp
@@ -112,6 +112,12 @@ namespace model {
 
     void resetPartofTotalFloorArea();
 
+    bool setVolume(double volume);
+
+    void autocalculateVolume();
+
+    void resetVolume();
+
     //@}
     /** @name Other */
 
@@ -280,6 +286,10 @@ namespace model {
     /// Does not include space multiplier in calculation.
     double volume() const;
 
+    bool isVolumeDefaulted() const;
+
+    bool isVolumeAutocalculated() const;
+
     /** Returns the number of people in the space (people). */
     /// Does not include space multiplier in calculation.
     /// Does include people multiplier in calculation.
diff --git a/src/model/Space_Impl.hpp b/src/model/Space_Impl.hpp
index 803d4c2d0aa..cd9f0ab4d90 100644
--- a/src/model/Space_Impl.hpp
+++ b/src/model/Space_Impl.hpp
@@ -162,6 +162,12 @@ namespace model {
 
       void resetPartofTotalFloorArea();
 
+      bool setVolume(double volume);
+
+      void autocalculateVolume();
+
+      void resetVolume();
+
       //@}
 
       /// Returns the space type for this space.
@@ -319,6 +325,10 @@ namespace model {
 
       double volume() const;
 
+      bool isVolumeDefaulted() const;
+
+      bool isVolumeAutocalculated() const;
+
       double numberOfPeople() const;
 
       bool setNumberOfPeople(double numberOfPeople);


Test patch:
diff --git a/src/model/test/Space_GTest.cpp b/src/model/test/Space_GTest.cpp
index 9d4f93bdbfb..fad604d1907 100644
--- a/src/model/test/Space_GTest.cpp
+++ b/src/model/test/Space_GTest.cpp
@@ -2782,6 +2782,57 @@ TEST_F(ModelFixture, ExposedPerimeter) {
   }
 }
 
+TEST_F(ModelFixture, Space_setVolume) {
+  // create from floor print
+  Model model;
+  Point3dVector floorPrint;
+  floorPrint.push_back(Point3d(0, 10, 0));
+  floorPrint.push_back(Point3d(10, 10, 0));
+  floorPrint.push_back(Point3d(10, 0, 0));
+  floorPrint.push_back(Point3d(0, 0, 0));
+  boost::optional<Space> ospace = Space::fromFloorPrint(floorPrint, 3.6, model);
+  ASSERT_TRUE(ospace);
+  Space space = *ospace;
+
+  // check dimensions
+  EXPECT_TRUE(space.isVolumeAutocalculated());
+  EXPECT_TRUE(space.isVolumeDefaulted());
+  EXPECT_DOUBLE_EQ(100.0, space.floorArea());
+  EXPECT_DOUBLE_EQ(360.0, space.volume());
+  EXPECT_DOUBLE_EQ(144.0, space.exteriorWallArea());
+  EXPECT_DOUBLE_EQ(244.0, space.exteriorArea());  // ground does not count
+
+  // set volume
+  EXPECT_TRUE(space.setVolume(365.0));
+  EXPECT_FALSE(space.isVolumeAutocalculated());
+  EXPECT_FALSE(space.isVolumeDefaulted());
+  EXPECT_DOUBLE_EQ(100.0, space.floorArea());
+  EXPECT_DOUBLE_EQ(365.0, space.volume());
+  EXPECT_DOUBLE_EQ(144.0, space.exteriorWallArea());
+  EXPECT_DOUBLE_EQ(244.0, space.exteriorArea());  // ground does not count
+
+  // reset volume
+  space.resetVolume();
+  EXPECT_TRUE(space.isVolumeAutocalculated());
+  EXPECT_TRUE(space.isVolumeDefaulted());
+  EXPECT_DOUBLE_EQ(100.0, space.floorArea());
+  EXPECT_DOUBLE_EQ(360.0, space.volume());
+  EXPECT_DOUBLE_EQ(144.0, space.exteriorWallArea());
+  EXPECT_DOUBLE_EQ(244.0, space.exteriorArea());  // ground does not count
+
+  // autocalculate volume
+  EXPECT_TRUE(space.setVolume(370.0));
+  EXPECT_FALSE(space.isVolumeAutocalculated());
+  EXPECT_FALSE(space.isVolumeDefaulted());
+  space.autocalculateVolume();
+  EXPECT_TRUE(space.isVolumeAutocalculated());
+  EXPECT_FALSE(space.isVolumeDefaulted());
+  EXPECT_DOUBLE_EQ(100.0, space.floorArea());
+  EXPECT_DOUBLE_EQ(360.0, space.volume());
+  EXPECT_DOUBLE_EQ(144.0, space.exteriorWallArea());
+  EXPECT_DOUBLE_EQ(244.0, space.exteriorArea());  // ground does not count
+}
+
 /*****************************************************************************************************************************************************
 *                                                           D I S A B L E D    T E S T S                                                            *
 *****************************************************************************************************************************************************/