Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4749

joseph-robertson · 2022-11-28T21:51:41Z

Pull request overview

Fixes Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4747
Checklist:
- Update idd
- Update model files
- Update ft file
- Update model tests
- Add vt

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

Made a couple of very minor changes. All good. Once CI reports success, we can merge. Thanks @joseph-robertson

jmarrec · 2022-12-09T12:40:06Z

resources/model/OpenStudio.idd

+  N1 , \field No Load Supply Air Flow Rate Ratio
+       \required-field
+       \type real
+       \minimum 0
+       \maximum 1
+       \note Used to define the no load operating air flow rate when the system fan
+       \note is specified to operate continuously.


Ok, default of 1 is changed to be a required-field, 👍

jmarrec · 2022-12-09T12:41:15Z

src/model/UnitarySystemPerformanceMultispeed.cpp

+    double UnitarySystemPerformanceMultispeed_Impl::noLoadSupplyAirflowRateRatio() const {
+      boost::optional<double> value = getDouble(OS_UnitarySystemPerformance_MultispeedFields::NoLoadSupplyAirFlowRateRatio, true);
+      OS_ASSERT(value);
+      return value.get();
+    }
+
+    bool UnitarySystemPerformanceMultispeed_Impl::setNoLoadSupplyAirflowRateRatio(double noLoadSupplyAirflowRateRatio) {
+      bool result = setDouble(OS_UnitarySystemPerformance_MultispeedFields::NoLoadSupplyAirFlowRateRatio, noLoadSupplyAirflowRateRatio);
+      return result;
+    }


A new getter and a setter (no reset), all good.

jmarrec · 2022-12-09T12:41:29Z

src/model/UnitarySystemPerformanceMultispeed.cpp

+    bool ok = setNoLoadSupplyAirflowRateRatio(1.0);
+    OS_ASSERT(ok);


Ctor sets the required field, good.

jmarrec · 2022-12-09T12:42:00Z

src/energyplus/ForwardTranslator/ForwardTranslateUnitarySystemPerformanceMultispeed.cpp

+    // No Load Supply Air Flow Rate Ratio
+    sysPerf.setDouble(UnitarySystemPerformance_MultispeedFields::NoLoadSupplyAirFlowRateRatio, modelObject.noLoadSupplyAirflowRateRatio());
+


FT sets the field, which is required, so no if optional, 👍

ideally a FT test would ensure this works appropriately.

jmarrec · 2022-12-09T12:42:13Z

src/model/test/UnitarySystemPerformanceMultispeed_GTest.cpp

+  // No Load
+  EXPECT_EQ(1.0, testObject.noLoadSupplyAirflowRateRatio());
+  EXPECT_TRUE(testObject.setNoLoadSupplyAirflowRateRatio(0.5));
+  EXPECT_EQ(0.5, testObject.noLoadSupplyAirflowRateRatio());


No problem here

jmarrec · 2022-12-09T12:49:51Z

src/osversion/VersionTranslator.cpp

+          auto value = object.getString(i);
+          if (value) {


Suggested change

auto value = object.getString(i);

if (value) {

if ((value = object.getString(i))) {

jmarrec · 2022-12-09T12:50:44Z

src/osversion/VersionTranslator.cpp

+            if (i < 3) {
+              newObject.setString(i, value.get());
+            } else {
+              newObject.setString(i + 1, value.get());
+            }
+          }
+        }
+
+        newObject.setDouble(3, 1.0);
+
+        m_refactored.push_back(RefactoredObjectData(object, newObject));
+        ss << newObject;


No problem here, all good.

jmarrec · 2022-12-09T12:51:39Z

src/osversion/test/VersionTranslator_GTest.cpp

+  EXPECT_EQ("Unitary System Performance Multispeed 1", perf.getString(OS_UnitarySystemPerformance_MultispeedFields::Name).get());
+  EXPECT_EQ("No", perf.getString(OS_UnitarySystemPerformance_MultispeedFields::SingleModeOperation).get());
+  EXPECT_EQ(1.0, perf.getDouble(OS_UnitarySystemPerformance_MultispeedFields::NoLoadSupplyAirFlowRateRatio).get());


Undefined OS_UnitarySystemPerformance_MultispeedFields

Hmm, these weren't working?

unless you #include the specific Object_FieldEnums.hxx it doesn't no. I'm torn about whether we want to start using the FieldEnums or not... it's going to create problems where we need to update them even if it's just the casing that changes, so I chose not to

ci-commercialbuildings · 2022-12-09T13:08:55Z

CI Results for 4843352:

Ubuntu 18.04:
- OpenStudio-3.5.1-alpha+4843352453-Ubuntu-18.04.deb
- OpenStudio-3.5.1-alpha+4843352453-Ubuntu-18.04.tar.gz
Ubuntu 20.04:
Darwin:
- OpenStudio-3.5.1-alpha+4843352453-Darwin-x86_64.dmg
- OpenStudio-3.5.1-alpha+4843352453-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.5.1-alpha+4843352453-Windows.exe
- OpenStudio-3.5.1-alpha+4843352453-Windows.tar.gz
Code Coverage:

jmarrec · 2022-12-09T13:20:18Z

@joseph-robertson I don't have jenkins access here, could you check why the Windows runner is failing please?

joseph-robertson added 3 commits November 28, 2022 14:49

Add no load field to idd.

84e975c

Update model files and test.

5abe335

Update ft file.

50958de

joseph-robertson added component - Model Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Nov 28, 2022

joseph-robertson self-assigned this Nov 28, 2022

Fix types.

d2dc628

joseph-robertson changed the base branch from develop to bump-vt November 29, 2022 15:41

joseph-robertson added 6 commits November 29, 2022 08:42

Merge branch 'bump-vt' into unitarysysperf-noload

d877023

Update new method name for consistency.

1186b69

Add vt.

03d5d76

Add vt test.

1c255c6

Clean up new method name.

d9374b5

Newline.

539f5bf

Base automatically changed from bump-vt to develop November 30, 2022 16:19

Merge branch 'develop' into unitarysysperf-noload

6cab8d6

joseph-robertson marked this pull request as ready for review November 30, 2022 18:20

joseph-robertson requested a review from jmarrec November 30, 2022 18:20

joseph-robertson and others added 3 commits December 8, 2022 15:13

Merge branch 'develop' into unitarysysperf-noload

6d8d1bc

Fix build error

62fc831

Minor adjustment

4843352

jmarrec approved these changes Dec 9, 2022

View reviewed changes

jmarrec merged commit bd130bb into develop Dec 9, 2022

jmarrec deleted the unitarysysperf-noload branch December 9, 2022 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4749

Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4749

joseph-robertson commented Nov 28, 2022 •

edited

Loading

jmarrec left a comment

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022 •

edited

Loading

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

jmarrec Dec 9, 2022

joseph-robertson Dec 9, 2022

jmarrec Dec 20, 2022

ci-commercialbuildings commented Dec 9, 2022 •

edited

Loading

jmarrec commented Dec 9, 2022

		bool ok = setNoLoadSupplyAirflowRateRatio(1.0);
		OS_ASSERT(ok);

		// No Load Supply Air Flow Rate Ratio
		sysPerf.setDouble(UnitarySystemPerformance_MultispeedFields::NoLoadSupplyAirFlowRateRatio, modelObject.noLoadSupplyAirflowRateRatio());

	auto value = object.getString(i);
	if (value) {
	if ((value = object.getString(i))) {

Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4749

Support "No Load Supply Air Flow Rate Ratio" on UnitarySystemPerformance:Multispeed #4749

Conversation

joseph-robertson commented Nov 28, 2022 • edited Loading

Pull request overview

Pull Request Author

Review Checklist

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

jmarrec Dec 9, 2022 • 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

ci-commercialbuildings commented Dec 9, 2022 • edited Loading

jmarrec commented Dec 9, 2022

joseph-robertson commented Nov 28, 2022 •

edited

Loading

jmarrec Dec 9, 2022 •

edited

Loading

ci-commercialbuildings commented Dec 9, 2022 •

edited

Loading