AirLoopHVACUnitarySystem set Method During XXX Operation - option #4905

jmarrec · 2023-06-01T11:19:19Z

Pull request overview

Cleanup the DuringCooling API. One break, the return of supplyAirFlowRateMethodDuringCooling, which can be avoided (keep returning optional even if always initialized)

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

…RateMethodDuringCooling, which can be avoided (keep returning optional even if always initialized)

jmarrec · 2023-06-01T11:20:53Z

src/model/AirLoopHVACUnitarySystem.hpp

@@ -130,7 +132,7 @@ namespace model {
    boost::optional<HVACComponent> supplementalHeatingCoil() const;

    /** In EnergyPlus 8.3.0 and above this property maps to the EnergyPlus field "Cooling Supply Air Flow Rate Method" **/
-    boost::optional<std::string> supplyAirFlowRateMethodDuringCoolingOperation() const;
+    std::string supplyAirFlowRateMethodDuringCoolingOperation() const;


Could avoid this API break by keeping it as optional<string> even though it'll always be set.

jmarrec · 2023-06-01T11:21:30Z

src/model/AirLoopHVACUnitarySystem.cpp

+  bool AirLoopHVACUnitarySystem::setSupplyAirFlowRateMethodDuringCoolingOperation(const std::string& supplyAirFlowRateMethodDuringCoolingOperation) {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+    return false;
+  }
+
+  void AirLoopHVACUnitarySystem::resetSupplyAirFlowRateMethodDuringCoolingOperation() {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+  }
+
+  void AirLoopHVACUnitarySystem::resetSupplyAirFlowRateDuringCoolingOperation() {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+  }
+
+  void AirLoopHVACUnitarySystem::resetSupplyAirFlowRatePerFloorAreaDuringCoolingOperation() {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+  }
+
+  void AirLoopHVACUnitarySystem::resetFractionofAutosizedDesignCoolingSupplyAirFlowRate() {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+  }
+
+  void AirLoopHVACUnitarySystem::resetDesignSupplyAirFlowRatePerUnitofCapacityDuringCoolingOperation() {
+    log_deprecation_and_throw_if_time_to_remove(BOOST_CURRENT_FUNCTION);
+  }


All of these become no-op deprecated functions

@jmarrec Can most of this new stuff be removed since (I believe) it pre-dates #4912?

Yes, replace with something like

DEPRECATED_AT_MSG(3, 7, 0, "Use setSupplyAirFlowRateDuringCoolingOperation instead.");

jmarrec · 2023-06-01T11:22:00Z

src/model/AirLoopHVACUnitarySystem_Impl.hpp

@@ -298,26 +299,23 @@ namespace model {

      void resetSupplementalHeatingCoil();

-      bool setSupplyAirFlowRateMethodDuringCoolingOperation(boost::optional<std::string> supplyAirFlowRateMethodDuringCoolingOperation);
+      bool setSupplyAirFlowRateMethodDuringCoolingOperation(const std::string& supplyAirFlowRateMethodDuringCoolingOperation);


The Impl side is ugly. cleanup the use of unecessary optionals

jmarrec · 2023-06-01T11:22:51Z

src/model/AirLoopHVACUnitarySystem.cpp

+    bool AirLoopHVACUnitarySystem_Impl::hasCoolingCoil() const {
+      return !isEmpty(OS_AirLoopHVAC_UnitarySystemFields::CoolingCoilName);
+    }


I am NOT using it right now. But if we wanted to disallow calling setSupplyAirFlowRateXXXDuringCoolingOperation to the case where there IS a cooling coil, we'd use that

jmarrec · 2023-06-01T11:23:17Z

src/model/AirLoopHVACUnitarySystem.cpp

+    bool AirLoopHVACUnitarySystem_Impl::setCoolingCoil(const HVACComponent& coolingCoil) {
+      const bool result = setPointer(OS_AirLoopHVAC_UnitarySystemFields::CoolingCoilName, coolingCoil.handle());
+      if (openstudio::istringEqual("None", supplyAirFlowRateMethodDuringCoolingOperation())) {
+        autosizeSupplyAirFlowRateDuringCoolingOperation();
+        OS_ASSERT(setSupplyAirFlowRateMethodDuringCoolingOperation("SupplyAirFlowRate"));


set Cooling Coil: set to SupplyAirFlowRate.

jmarrec · 2023-06-01T11:24:13Z

src/model/AirLoopHVACUnitarySystem.cpp

+      const bool result = setString(OS_AirLoopHVAC_UnitarySystemFields::CoolingCoilName, "");
      OS_ASSERT(result);
+      OS_ASSERT(setSupplyAirFlowRateMethodDuringCoolingOperation("None"));
+      resetSupplyAirFlowRateDuringCoolingOperation();
+      resetSupplyAirFlowRatePerFloorAreaDuringCoolingOperation();
+      resetFractionofAutosizedDesignCoolingSupplyAirFlowRate();
+      resetDesignSupplyAirFlowRatePerUnitofCapacityDuringCoolingOperation();


ResetCoolingCoil: set to None, clear out of the other fields.

I'm not sure whether we want to reset all fields at all times or not... Only ONE field will be used in the end, but it may be slightly annoying for people if they are testing things out.

jmarrec · 2023-06-01T11:25:42Z

src/model/AirLoopHVACUnitarySystem.cpp

+    bool AirLoopHVACUnitarySystem_Impl::setSupplyAirFlowRateDuringCoolingOperation(double supplyAirFlowRateDuringCoolingOperation) {
+      const bool result =
+        setDouble(OS_AirLoopHVAC_UnitarySystemFields::SupplyAirFlowRateDuringCoolingOperation, supplyAirFlowRateDuringCoolingOperation);
+      OS_ASSERT(setSupplyAirFlowRateMethodDuringCoolingOperation("SupplyAirFlowRate"));
+      // resetSupplyAirFlowRateDuringCoolingOperation();
+      resetSupplyAirFlowRatePerFloorAreaDuringCoolingOperation();
+      resetFractionofAutosizedDesignCoolingSupplyAirFlowRate();
+      resetDesignSupplyAirFlowRatePerUnitofCapacityDuringCoolingOperation();


For each of these setters, you set the field + the method, reset all fields (again, not sure we want to reset all)

joseph-robertson · 2023-06-14T16:09:02Z

@jmarrec Seems easier to update the idd Method fields with \default None, no? And then change the return type of the getters to not be optional?

…itary-sys-opmethod2

jmarrec · 2023-06-22T12:06:45Z

@joseph-robertson I'm not sure I follow. Which fields do you want to set to None?

joseph-robertson · 2023-06-22T14:36:53Z

@jmarrec Introduce \default None to the following fields:

Supply Air Flow Rate Method During Cooling Operation
Supply Air Flow Rate Method During Heating Operation

Then you wouldn't need that special code inside supplyAirFlowRateMethodDuringCoolingOperation().

jmarrec · 2023-06-29T14:17:42Z

Yeah I think that makes sense. Up to you really.

…itary-sys-opmethod2

AirLoopHVACUnitarySystem set Method During XXX Operation - option, 2

ci-commercialbuildings · 2023-07-14T15:22:40Z

CI Results for 36a9a7d:

Ubuntu 20.04:
Ubuntu 22.04:
- OpenStudio-3.7.0-alpha+36a9a7d971-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.7.0-alpha+36a9a7d971-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.7.0-alpha+36a9a7d971-Darwin-x86_64.dmg
- OpenStudio-3.7.0-alpha+36a9a7d971-Darwin-x86_64.tar.gz
Windows:
Code Coverage:

Cleanup the DuringCooling API. One break, the return of supplyAirFlow…

9405db6

…RateMethodDuringCooling, which can be avoided (keep returning optional even if always initialized)

jmarrec added Developer Issue component - HVAC component - Model labels Jun 1, 2023

jmarrec requested a review from joseph-robertson June 1, 2023 11:19

jmarrec self-assigned this Jun 1, 2023

jmarrec mentioned this pull request Jun 1, 2023

AirLoopHVACUnitarySystem set Method During XXX Operation #4873

Merged

19 tasks

jmarrec commented Jun 1, 2023

View reviewed changes

Merge branch 'airloop-hvac-unitary-sys-opmethod' into airloop-hvac-un…

5d93c8d

…itary-sys-opmethod2

joseph-robertson and others added 4 commits July 12, 2023 09:12

Merge branch 'airloop-hvac-unitary-sys-opmethod' into airloop-hvac-un…

3728488

…itary-sys-opmethod2

Add None default to idd.

08858b4

Simplify method getter after introducing None default.

fa7543a

Merge pull request #4927 from NREL/airloop-hvac-unitary-sys-opmethod3

af26872

AirLoopHVACUnitarySystem set Method During XXX Operation - option, 2

joseph-robertson added Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. severity - Minor Bug and removed Developer Issue labels Jul 13, 2023

joseph-robertson added 10 commits July 13, 2023 07:22

Add None default for HeatingOperation.

d3d1f3d

Updates for new os_deprecated method args.

b54af03

Update ft for heating.

dd0bd1a

Update all heating methods.

01ee57b

Formatting.

da09bd9

Clean up minor typos.

79b6ca3

More case typos.

57d142d

Update model tests.

fd3d025

More case typos.

1413329

Stub deprecated_at_msg in cpp.

2cef40d

joseph-robertson added 2 commits July 14, 2023 07:49

Update deprecated messages.

df44eab

Include deprecatedhelpers hpp.

36a9a7d

joseph-robertson approved these changes Jul 14, 2023

View reviewed changes

joseph-robertson merged commit b053f33 into airloop-hvac-unitary-sys-opmethod Jul 14, 2023
2 of 4 checks passed

joseph-robertson deleted the airloop-hvac-unitary-sys-opmethod2 branch July 14, 2023 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AirLoopHVACUnitarySystem set Method During XXX Operation - option #4905

AirLoopHVACUnitarySystem set Method During XXX Operation - option #4905

jmarrec commented Jun 1, 2023

jmarrec Jun 1, 2023

jmarrec Jun 1, 2023

joseph-robertson Jul 13, 2023

jmarrec Jul 14, 2023

jmarrec Jun 1, 2023

jmarrec Jun 1, 2023

jmarrec Jun 1, 2023

jmarrec Jun 1, 2023

jmarrec Jun 1, 2023

joseph-robertson commented Jun 14, 2023

jmarrec commented Jun 22, 2023

joseph-robertson commented Jun 22, 2023 •

edited

Loading

jmarrec commented Jun 29, 2023

ci-commercialbuildings commented Jul 14, 2023 •

edited

Loading

AirLoopHVACUnitarySystem set Method During XXX Operation - option #4905

AirLoopHVACUnitarySystem set Method During XXX Operation - option #4905

Conversation

jmarrec commented Jun 1, 2023

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

joseph-robertson commented Jun 14, 2023

jmarrec commented Jun 22, 2023

joseph-robertson commented Jun 22, 2023 • edited Loading

jmarrec commented Jun 29, 2023

ci-commercialbuildings commented Jul 14, 2023 • edited Loading

joseph-robertson commented Jun 22, 2023 •

edited

Loading

ci-commercialbuildings commented Jul 14, 2023 •

edited

Loading