ControllerOutdoorAir: new Economizer Operation Staging field #4948

joseph-robertson · 2023-08-29T17:22:45Z

Pull request overview

Support new "Economizer Operation Staging" field for Controller:OutdoorAir.

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 · 2023-09-12T13:18:25Z

resources/model/OpenStudio.idd

+  A21; \field Economizer Operation Staging
+       \type choice
+       \key EconomizerFirst
+       \key InterlockedWithMechanicalCooling
+       \default InterlockedWithMechanicalCooling


As usual, torn on whether we should make this required-field or not... The field above isn't.

@kbenne

I think because it is a new field we don't risk breaking an API so required field is better. That is, fewer options is better and I don't see much downside except perhaps being a little inconsistent with similar cases.

jmarrec · 2023-09-12T13:20:42Z

src/model/ControllerOutdoorAir.hpp

+    boost::optional<std::string> getEconomizerOperationStaging() const;
+    bool setEconomizerOperationStaging(const std::string& v);


Pffff. Another one where the existing API doesn't follow the convention.

This should be std::string economizerOperationStaging() const. Not getXXXX. And it has a default (at least currently), so no need to return an optional.
And there should be a isEconomizerOperationStagingDefaulted and a resetEconomizerOperationStaging if we do intend to have a default instead of making it required-field.

@kbenne thoughts please.

It seems like we can keep (and depricate) the existing "get" method and add the new (correct) std::string economizerOperationStaging() const. Is that right?

No deprecation necessary - this is a new field. I'll follow the convention for required-field (which is "correct" but inconsistent with other fields on this object).

This specific comment was more about fixing the existing API, not the new field.
You don't have to do it here, we have lots to do at the moment. But we should file an issue to fix it later if it's not done here.

Fix inconsistent API in ControllerOutdoorAir #4981

joseph-robertson · 2023-09-25T17:14:19Z

src/osversion/VersionTranslator.cpp

+      } else if (iddname == "OS:Controller:OutdoorAir") {
+
+        // 1 Field has been added from 3.6.1 to 3.7.0:
+        // -------------------------------------------
+        // * Economizer Operation Staging * 27
+        auto iddObject = idd_3_7_0.getObject(iddname);
+        IdfObject newObject(iddObject.get());
+
+        for (size_t i = 0; i < object.numFields(); ++i) {
+          if ((value = object.getString(i))) {
+            newObject.setString(i, value.get());
+          }
+        }
+
+        newObject.setString(27, "InterlockedWithMechanicalCooling");


"Economizer Operation Staging" is required, so we need VT...

ci-commercialbuildings · 2023-09-25T18:22:59Z

CI Results for 0a5e651:

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

jmarrec

LGTM

jmarrec · 2023-09-26T11:14:21Z

resources/model/OpenStudio.idd

+  A21; \field Economizer Operation Staging
+       \type choice
+       \key EconomizerFirst
+       \key InterlockedWithMechanicalCooling
+       \required-field


Required field, all good

jmarrec · 2023-09-26T11:14:38Z

src/model/ControllerOutdoorAir.hpp

+    std::string economizerOperationStaging() const;
+    bool setEconomizerOperationStaging(const std::string& v);


jmarrec · 2023-09-26T11:15:05Z

src/osversion/test/VersionTranslator_GTest.cpp

+  EXPECT_TRUE(coa.isEmpty(24));                                            // High Humidity Outdoor Air Flow Ratio
+  EXPECT_TRUE(coa.isEmpty(25));                                            // Control High Indoor Humidity Based on Outdoor Humidity Ratio
+  EXPECT_EQ("BypassWhenWithinEconomizerLimits", coa.getString(26).get());  // Heat Recovery Bypass Control Type
+  EXPECT_EQ("InterlockedWithMechanicalCooling", coa.getString(27).get());  // Economizer Operation Staging


jmarrec · 2023-09-26T11:15:51Z

src/model/ControllerOutdoorAir.cpp

@@ -597,6 +604,7 @@ namespace model {
    setString(OS_Controller_OutdoorAirFields::HighHumidityOutdoorAirFlowRatio, "");
    setString(OS_Controller_OutdoorAirFields::ControlHighIndoorHumidityBasedonOutdoorHumidityRatio, "");
    setHeatRecoveryBypassControlType("BypassWhenWithinEconomizerLimits");
+    setEconomizerOperationStaging("InterlockedWithMechanicalCooling");


jmarrec · 2023-09-26T11:15:58Z

src/energyplus/ForwardTranslator/ForwardTranslateControllerOutdoorAir.cpp

@@ -173,6 +173,9 @@ namespace energyplus {
      idfObject.setString(openstudio::Controller_OutdoorAirFields::HeatRecoveryBypassControlType, *s);
    }

+    // EconomizerOperationStaging
+    idfObject.setString(openstudio::Controller_OutdoorAirFields::EconomizerOperationStaging, modelObject.economizerOperationStaging());


jmarrec · 2023-09-26T11:16:27Z

src/energyplus/ReverseTranslator/ReverseTranslateControllerOutdoorAir.cpp

+    s = workspaceObject.getString(Controller_OutdoorAirFields::EconomizerOperationStaging);
+    if (s) {
+      mo.setEconomizerOperationStaging(s.get());
+    }


If not present, uses the Ctor default, no problem here

joseph-robertson added 4 commits August 29, 2023 10:20

Add new field for controller outdoor air.

93ff53e

Update source files for new controller outdoor air field.

50a6b38

Update ft and rt for new controller outdoor air field.

80c90be

Add model test for new controller outdoor air field.

f045c29

joseph-robertson added this to the OpenStudio SDK 3.7.0 milestone Aug 29, 2023

joseph-robertson self-assigned this Aug 29, 2023

joseph-robertson mentioned this pull request Aug 29, 2023

Update to EnergyPlus 23.2.0 #4934

Closed

26 tasks

joseph-robertson added Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. IDDChange labels Aug 29, 2023

joseph-robertson added 2 commits August 29, 2023 13:41

Formatting.

c5295c9

Merge branch 'v23.2.0-IOFreeze' into controller-outdoor-air

eec870d

jmarrec reviewed Sep 12, 2023

View reviewed changes

joseph-robertson added 5 commits September 25, 2023 09:28

Merge branch 'v23.2.0-IOFreeze' into controller-outdoor-air

53bc50c

Make new field a required field.

d8bd66e

Follow convention of required getter.

c75430a

Add vt since this new field is required.

dae4e9e

Formatting.

2cd649e

joseph-robertson commented Sep 25, 2023

View reviewed changes

Fix return type in cpp.

0a5e651

joseph-robertson marked this pull request as ready for review September 25, 2023 18:29

joseph-robertson requested a review from jmarrec September 25, 2023 18:29

jmarrec mentioned this pull request Sep 26, 2023

Fix inconsistent API in ControllerOutdoorAir #4981

Open

jmarrec approved these changes Sep 26, 2023

View reviewed changes

jmarrec merged commit 455fc44 into v23.2.0-IOFreeze Sep 26, 2023
3 of 4 checks passed

jmarrec deleted the controller-outdoor-air branch September 26, 2023 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ControllerOutdoorAir: new Economizer Operation Staging field #4948

ControllerOutdoorAir: new Economizer Operation Staging field #4948

joseph-robertson commented Aug 29, 2023 •

edited

Loading

jmarrec Sep 12, 2023

kbenne Sep 19, 2023

jmarrec Sep 12, 2023

kbenne Sep 25, 2023

joseph-robertson Sep 25, 2023

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023 •

edited

Loading

joseph-robertson Sep 25, 2023

ci-commercialbuildings commented Sep 25, 2023 •

edited

Loading

jmarrec left a comment

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023

jmarrec Sep 26, 2023

		boost::optional<std::string> getEconomizerOperationStaging() const;
		bool setEconomizerOperationStaging(const std::string& v);

		std::string economizerOperationStaging() const;
		bool setEconomizerOperationStaging(const std::string& v);

ControllerOutdoorAir: new Economizer Operation Staging field #4948

ControllerOutdoorAir: new Economizer Operation Staging field #4948

Conversation

joseph-robertson commented Aug 29, 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

jmarrec Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Sep 25, 2023 • edited Loading

jmarrec 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-robertson commented Aug 29, 2023 •

edited

Loading

jmarrec Sep 26, 2023 •

edited

Loading

ci-commercialbuildings commented Sep 25, 2023 •

edited

Loading