V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) #4968

jmarrec · 2023-09-18T15:03:26Z

Pull request overview

Fixes Partially the Update to EnergyPlus 23.2.0 #4934
Fuel Type enum changes (OtherEquipment, Exterior:FuelEquipment, ZoneHVAC:HybridUnitaryHVAC, etc.)
- "DistrictHeating" to "DistrictHeatingWater"
- "Steam" to "DistrictHeatingSteam"

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

…or ExteriorFuelEquipment (already done of OpeNStudio.idd)

…eatingSteam in IDD + DataEnum FuelType

…able and EMS:MeteredOutputVariable

…or and Output:Meter

jmarrec · 2023-09-18T15:04:48Z

resources/model/OpenStudio.idd

+        \key DistrictHeatingSteam
+        \key DistrictHeatingWater


Renames in OpenStudio.idd

jmarrec · 2023-09-18T15:04:55Z

src/model/DistrictCooling.cpp

+        "District Cooling Water Rate",
+        "District Cooling Water Energy",
+        "District Cooling Water Inlet Temperature",
+        "District Cooling Water Outlet Temperature",
+        "District Cooling Water Mass Flow Rate",
+      };


Update out vars

jmarrec · 2023-09-18T15:05:27Z

src/utilities/data/DataEnums.hpp

+  ((Steam)(DistrictHeatingSteam))
  ((DistrictCooling))
-  ((DistrictHeating))
+  ((DistrictHeating)(DistrictHeatingWater))


Aliases on FuelType Enum. SHould always use valueDescription() to interface with E+ now

jmarrec · 2023-09-18T15:05:46Z

src/utilities/data/DataEnums.hpp

 /** \class FuelType
- *  \brief EnergyPlus meterable fuel types
- *  \details See the OPENSTUDIO_ENUM documentation in utilities/core/Enum.hpp. The actual
+ *  \brief EnergyPlus meterable fuel types.
+ *  \details Use the valueDescription() for interfacing with EnergyPlus or setting an IDD field.
+ *
+ *  See the OPENSTUDIO_ENUM documentation in utilities/core/Enum.hpp. The actual


Clarify that you need valueDescription for E+ in docstring

jmarrec · 2023-09-18T15:08:04Z

src/model/MeterCustom.hpp

+    bool setFuelType(const FuelType& fuelType);
    bool setFuelType(const std::string& fuelType);


Ok, so for all types (ALMOST) that received a change, I implement an overload setFuelType. This overload is called by the setFuelType(std::string& one)

I do not do it for PythonPlugin:OutputVariable and EMS:MeteredOutputVariable.
The reason is that they support a ton more variables (and are less common objects). See this nice table I made here: https://docs.google.com/spreadsheets/d/1ZjLABsEOAYmAyrrI6iM1JMJi6uRYBMWHlu3AUcx40iE/edit#gid=518449229

They support all of these.

ENERGYTRANSFER MAINSWATERSUPPLY ONSITEWATERPRODUCED RAINWATERCOLLECTED SOLARAIRHEATING SOLARWATERHEATING CONDENSATEWATERCOLLECTED WATERUSE WELLWATERDRAWN

jmarrec · 2023-09-18T15:09:04Z

src/model/MeterCustom.cpp

+  bool MeterCustom::setFuelType(const std::string& fuelType) {
+    // Special case for custom meters, the only ones that have "Generic" as a possibility
+    if (openstudio::istringEqual("Generic", fuelType)) {
+      return this->setString(OS_Meter_CustomFields::FuelType, "Generic");
+    }
+    try {
+      return setFuelType(FuelType{fuelType});
+    } catch (std::runtime_error& e) {
+      LOG(Debug, e.what());
+      return false;
+    }
+  }


So here is the setFuelType(std::string) one. As you can see, here I handle a special case, otherwise I defer to setFuelType(FuelType)

This allows me to call both

setFuelType("Steam") and setFuelType("DistrictHeatingSteam")

It's a special case because "Generic" is not added to DataEnums?

Yes. See the comment.

// Special case for custom meters, the only ones that have "Generic" as a possibility

I don't think we want to add Generic to FuelType enum just for this object.

jmarrec · 2023-09-18T15:09:17Z

src/model/OutputMeter.cpp

+      "Baseboard)?:?(Electricity|Gasoline|NaturalGas|Diesel|Coal|FuelOilNo1|FuelOilNo2|Propane|Water|DistrictHeatingSteam|DistrictCooling|"
+      "DistrictHeatingWater|OtherFuel1|OtherFuel2|EnergyTransfer)?:?(Facility|Building|HVAC|Zone|System|Plant)?:?([^:]*?)?$");


Update OutputMeter regex

jmarrec · 2023-09-18T15:09:35Z

src/osversion/VersionTranslator.cpp

+    // Making the map case-insentive by providing a Comparator `IstringCompare`
+    const std::map<std::string, std::string, openstudio::IstringCompare> replaceFuelTypesMap{{
+      {"Steam", "DistrictHeatingSteam"},
+      {"DistrictHeating", "DistrictHeatingWater"},
+      // Additionally, for UtilityBill, align the IDD choices to E+. This will also be covered by this
+      {"FuelOil_1", "FuelOilNo1"},
+      {"FuelOil_2", "FuelOilNo2"},
+      {"OtherFuel_1", "OtherFuel1"},
+      {"OtherFuel_2", "OtherFuel2"},
+    }};
+
+    const std::multimap<std::string, int> fuelTypeRenamesMap{{
+      {"OS:OtherEquipment", 6},                                // Fuel Type
+      {"OS:Exterior:FuelEquipment", 4},                        // Fuel Use Type
+      {"OS:WaterHeater:Mixed", 11},                            // Heater Fuel Type


jmarrec · 2023-09-18T15:09:52Z

src/osversion/test/3_7_0/test_vt_FuelTypeRenames.rb

jmarrec · 2023-09-18T15:10:06Z

src/osversion/test/VersionTranslator_GTest.cpp

+TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_fuelTypeRenames) {
+  openstudio::path osmPath = resourcesPath() / toPath("osversion/3_7_0/test_vt_FuelTypeRenames.osm");


Lots of VT testing

ci-commercialbuildings · 2023-09-18T19:14:03Z

CI Results for 821293a:

Ubuntu 20.04:
Ubuntu 22.04:
- OpenStudio-3.7.0-alpha+821293ae10-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.7.0-alpha+821293ae10-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.7.0-alpha+821293ae10-Darwin-x86_64.dmg
- OpenStudio-3.7.0-alpha+821293ae10-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.7.0-alpha+821293ae10-Windows.exe
- OpenStudio-3.7.0-alpha+821293ae10-Windows.tar.gz
Code Coverage:

joseph-robertson

This looks great. Just a few clarifying questions. I'd also suggest a brief high-level description of this PR in the description box.

joseph-robertson · 2023-09-18T19:39:36Z

resources/energyplus/ProposedEnergy+.idd

@@ -15410,7 +15410,6 @@ Exterior:FuelEquipment,
       \required-field
       \type choice
       \key Electricity
-       \key Water


So before, "Water" and "DistrictHeating" were redundant?

Or maybe "Water" was never mapped to?

Exterior:WaterEquipment is there for a reason. This is a change I'm upstreaming to E+ as well.

joseph-robertson · 2023-09-18T19:41:33Z

resources/model/OpenStudio.idd

@@ -35642,7 +35641,7 @@ OS:LifeCycleCost:UsePriceEscalation,
        \key ElectricitySurplusSold
        \key ElectricityNet
        \key NaturalGas
-        \key Steam
+        \key DistrictHeatingSteam


No "DistrictHeatingWater" here?

E+ doesn't support it yet, I'm trying to fix that in:

Fix #10220 - LifeCycleCost objects are using "Steam" not "DistrictHeatingSteam" EnergyPlus#10229

Wasn't sure if I should try to support it just yet when E+ doesn't.
If that PR is merged, when we received a new E+ package, we'll catch the update.

I added a specific note in #4934

joseph-robertson · 2023-09-18T19:42:03Z

resources/model/OpenStudio.idd

-       \key OtherFuel_2
+       \key DistrictHeatingWater
+       \key OtherFuel1
+       \key OtherFuel2


Hmm, how were these able to slip through the cracks?

UtilityBill actually uses FuelType and FuelType::valueDescription to write the E+ IDF, so it was working just fine already

explicit UtilityBill(const FuelType& fuelType, const Model& model);

I'm just aiming for consistency here.

joseph-robertson · 2023-09-18T19:49:21Z

src/model/ExteriorFuelEquipment.cpp

@@ -130,8 +131,8 @@ namespace model {
      OS_ASSERT(result);
    }

-    bool ExteriorFuelEquipment_Impl::setFuelType(const std::string& fuelType) {
-      bool result = setString(OS_Exterior_FuelEquipmentFields::FuelUseType, fuelType);
+    bool ExteriorFuelEquipment_Impl::setFuelType(const FuelType& fuelType) {


We should've been using FuelType before?

That's my opinion, yes. Cf #4940 (comment)

joseph-robertson · 2023-09-18T19:51:13Z

src/model/MeterCustom.cpp

+  bool MeterCustom::setFuelType(const std::string& fuelType) {
+    // Special case for custom meters, the only ones that have "Generic" as a possibility
+    if (openstudio::istringEqual("Generic", fuelType)) {
+      return this->setString(OS_Meter_CustomFields::FuelType, "Generic");
+    }
+    try {
+      return setFuelType(FuelType{fuelType});
+    } catch (std::runtime_error& e) {
+      LOG(Debug, e.what());
+      return false;
+    }
+  }


It's a special case because "Generic" is not added to DataEnums?

joseph-robertson · 2023-09-18T19:54:04Z

src/osversion/VersionTranslator.cpp

@@ -7897,6 +7898,83 @@ namespace osversion {
      {"OS:Coil:WaterHeating:AirToWaterHeatPump:Wrapped", 13},
    }};

+    // Making the map case-insentive by providing a Comparator `IstringCompare`


insensitive

I'll fix it in the next PR that rename OS:DistictHeating to OS:DistrictHeating:Water and takes in your DistrictHeating:Steam PR. Trying to save CI cycles.
(this is also a copy paste, I made that typo a long time ago :) )

joseph-robertson · 2023-09-18T19:57:45Z

src/osversion/VersionTranslator.cpp

@@ -8258,6 +8336,105 @@ namespace osversion {

        ss << object;

+      } else if (iddname == "OS:Boiler:HotWater") {


I'm confused why BoilerHotWater is here. Is this branch just slightly out of date?

I just moved it up in the if-else ladder.

Handle objects first, then do the lookup in the map things.

#4968 (comment)

jmarrec added 8 commits September 13, 2023 11:21

Remove duplicate choice in OS:WeatherProperty:SkyTemperature

10f407d

UtilityBill: Use FuelType valueDescription + Remove Water in E+ IDD f…

7625e71

…or ExteriorFuelEquipment (already done of OpeNStudio.idd)

Replace DistrictHeating=> DistrictHeatingWater and Steam => DistrictH…

0f890f9

…eatingSteam in IDD + DataEnum FuelType

Start VT

d46fe55

Model: add setFuelType(FuelType&) overloads. Missing PythonOutputVari…

3331e88

…able and EMS:MeteredOutputVariable

Adjust OutputMeter::meterRegex to match

cee66ae

Adjust OutputVariables, and VT for Output:Variable, EMS Sensor/Actuat…

ffaf437

…or and Output:Meter

Finish VT testing for Fuel Type renames

93715e6

jmarrec added component - Model component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Sep 18, 2023

jmarrec self-assigned this Sep 18, 2023

jmarrec mentioned this pull request Sep 18, 2023

Update to EnergyPlus 23.2.0 #4934

Closed

26 tasks

jmarrec requested a review from joseph-robertson September 18, 2023 15:04

jmarrec commented Sep 18, 2023

View reviewed changes

jmarrec added 3 commits September 18, 2023 18:09

EnvironementalImpactFactors: field name change, deprecated old methods

49433c8

Forgot one fuel rename in OS:UtilityBill

7e60e81

Update EndUseFuelType enum valueDescription

821293a

joseph-robertson changed the title ~~V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam~~ V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) Sep 18, 2023

joseph-robertson requested changes Sep 18, 2023

View reviewed changes

jmarrec merged commit 69021c4 into v23.2.0-IOFreeze Sep 19, 2023
0 of 4 checks passed

jmarrec deleted the v23.2.0-IOFreeze-fuel_types branch September 19, 2023 07:32

jmarrec added a commit that referenced this pull request Sep 19, 2023

Fix typos

dd24df3

#4968 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) #4968

V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) #4968

jmarrec commented Sep 18, 2023 •

edited

Loading

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

jmarrec Sep 18, 2023

ci-commercialbuildings commented Sep 18, 2023 •

edited

Loading

joseph-robertson left a comment

joseph-robertson Sep 18, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023 •

edited

Loading

jmarrec Sep 19, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023 •

edited

Loading

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023

joseph-robertson Sep 18, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023

joseph-robertson Sep 18, 2023

jmarrec Sep 19, 2023

		bool setFuelType(const FuelType& fuelType);
		bool setFuelType(const std::string& fuelType);

		"Baseboard)?:?(Electricity\|Gasoline\|NaturalGas\|Diesel\|Coal\|FuelOilNo1\|FuelOilNo2\|Propane\|Water\|DistrictHeatingSteam\|DistrictCooling\|"
		"DistrictHeatingWater\|OtherFuel1\|OtherFuel2\|EnergyTransfer)?:?(Facility\|Building\|HVAC\|Zone\|System\|Plant)?:?([^:]*?)?$");

		TEST_F(OSVersionFixture, update_3_6_1_to_3_7_0_fuelTypeRenames) {
		openstudio::path osmPath = resourcesPath() / toPath("osversion/3_7_0/test_vt_FuelTypeRenames.osm");

		@@ -8258,6 +8336,105 @@ namespace osversion {

		ss << object;

		} else if (iddname == "OS:Boiler:HotWater") {

V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) #4968

V23.2.0-IOFreeze - Fuel Type enum changes (DistrictHeatingWater / DistrictHeatingSteam) #4968

Conversation

jmarrec commented Sep 18, 2023 • edited Loading

Pull request overview

Pull Request Author

Review Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Sep 18, 2023 • edited Loading

joseph-robertson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec commented Sep 18, 2023 •

edited

Loading

ci-commercialbuildings commented Sep 18, 2023 •

edited

Loading

jmarrec Sep 19, 2023 •

edited

Loading

jmarrec Sep 19, 2023 •

edited

Loading