New Coil:*:WaterToAirHeatPump:EquationFit fields by joseph-robertson · Pull Request #4671 · NatLabRockies/OpenStudio

joseph-robertson · 2022-09-06T21:33:35Z

Pull request overview

Addresses Update to EnergyPlus 22.2.0 #4649
Coil:Heating:WaterToAirHeatPump:EquationFit
- Rated Entering Water Temperature
- Rated Entering Air Dry-Bulb Temperature
- Ratio of Rated Heating Capacity to Rated Cooling Capacity
Coil:Cooling:WaterToAirHeatPump:EquationFit
- Rated Entering Water Temperature
- Rated Entering Air Dry-Bulb Temperature
- Rated Entering Air Wet-Bulb Temperature
Updates model tests
Update FT
Add VT
Add new (missing) FT test file

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

@joseph-robertson Generally looks good. I would ideally like to see the FT code cleaned up and the testing (especially FT) see a couple of changes, but I don't feel super strongly on this so up to you if you want to do it or not.

src/model/CoilCoolingWaterToAirHeatPumpEquationFit.cpp

src/model/CoilHeatingWaterToAirHeatPumpEquationFit.cpp

src/model/test/CoilHeatingWaterToAirHeatPumpEquationFit_GTest.cpp

src/energyplus/ForwardTranslator/ForwardTranslateCoilCoolingWaterToAirHeatPumpEquationFit.cpp

jmarrec · 2022-09-19T14:01:30Z

src/energyplus/Test/ZoneHVACWaterToAirHeatPump_GTest.cpp

+  EXPECT_EQ(hp.nameString(), idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::Name).get());
+  EXPECT_EQ(sch.nameString(), idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::AvailabilityScheduleName).get());
+  EXPECT_NE("", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::AirInletNodeName, false).get());
+  EXPECT_NE("", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::AirOutletNodeName, false).get());
+  EXPECT_EQ("OutdoorAir:Mixer", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::OutdoorAirMixerObjectType, false).get());
+  EXPECT_NE("", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::OutdoorAirMixerName).get());
+  EXPECT_EQ("Autosize", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::CoolingSupplyAirFlowRate, false).get());


I like the fact that you added a new test

Maybe we should actually make it a full, targeted test. I see too much EXPECT_NE("", xxx) when I would like to see actual specific checks

Don't use EXPECT_EQ / EXPECT_NE with an empty string. Use EXPECT_TRUE(idf_hp.isEmpty(index)); instead

src/osversion/test/VersionTranslator_GTest.cpp

src/osversion/VersionTranslator.cpp

ci-commercialbuildings · 2022-09-19T23:22:00Z

CI Results for 7cc8064:

Ubuntu 18.04:
- OpenStudio-3.5.0-alpha+7cc8064870-Ubuntu-18.04.deb
- OpenStudio-3.5.0-alpha+7cc8064870-Ubuntu-18.04.tar.gz
Ubuntu 20.04:
Darwin:
- OpenStudio-3.5.0-alpha+7cc8064870-Darwin-x86_64.dmg
- OpenStudio-3.5.0-alpha+7cc8064870-Darwin-x86_64.tar.gz
Windows:
Code Coverage:

Conflicts: src/osversion/VersionTranslator.cpp src/osversion/test/VersionTranslator_GTest.cpp

jmarrec

FT test is highly suspicious.

jmarrec · 2022-09-20T07:39:44Z

src/energyplus/Test/ZoneHVACWaterToAirHeatPump_GTest.cpp

+  // Check Node Connections
+  // --- CC --- HC --- Fan --- supHC
+  EXPECT_EQ("", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::AirInletNodeName).get());
+  EXPECT_EQ("", idf_hp.getString(ZoneHVAC_WaterToAirHeatPumpFields::AirOutletNodeName).get());


I really doubt having nodes as blank is actually wanted here... Or am I missing something?

Taking ZoneWSHP_wDOAS.idf as an example, Nodes seem to be exactly like I expected: they match from one component to the next.

These two are actually the Zone Exhaust and Inlet Nodes

jmarrec · 2022-09-20T08:24:13Z

src/energyplus/Test/ZoneHVACWaterToAirHeatPump_GTest.cpp

+  EXPECT_EQ(idf_fan.getString(Fan_OnOffFields::AirOutletNodeName).get(),
+            idf_supHC.getString(Coil_Heating_ElectricFields::AirInletNodeName).get(),


Unterminated macro. Does that run on MSVC ?!?!

jmarrec · 2022-09-20T09:55:48Z

@joseph-robertson I've resolved conflicts and redid the FT testing to test everything field in both the high level object and the coils, and test that nodes are properly laid out, cf d04a448.

I'm merging without waiting.

joseph-robertson added 5 commits September 6, 2022 14:28

Update os idd.

5bfbb1e

Add model getters and setters.

f89da36

Update ft for new fields.

dbc0640

Update vt and add tests.

a2278c5

Formatting.

1ea7c95

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 Sep 6, 2022

joseph-robertson self-assigned this Sep 6, 2022

joseph-robertson mentioned this pull request Sep 6, 2022

Update to EnergyPlus 22.2.0 #4649

Closed

17 tasks

joseph-robertson changed the base branch from develop to v22.2.0-IOFreeze September 6, 2022 21:34

joseph-robertson added 7 commits September 6, 2022 16:08

Typo in new cpp setter.

d40d3e6

Clean up cpp files a bit.

2701257

Clean up setter methods.

e16c507

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

6d70223

Fix formatting in vt cpp.

7dd1240

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

eee96c5

Newline.

f37c7c5

tijcolem added this to the OpenStudio SDK 3.5.0 milestone Sep 9, 2022

joseph-robertson added 12 commits September 9, 2022 16:37

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

c0543ab

Fix vt and tests.

8bf1a05

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

e2125b3

Typo.

d40a8ad

Fix merge conflicts.

630ec7e

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

9fac3d9

Add new ft test for watertoairhp objects.

f35bdae

Formatting.

61801d1

Fill out new ft test fields.

3c59e26

Try test again.

125e979

Use isEmpty method for non translated fields.

cfd2d0b

Formatting.

ebecd2c

joseph-robertson marked this pull request as ready for review September 16, 2022 19:58

joseph-robertson requested a review from jmarrec September 16, 2022 19:58

jmarrec approved these changes Sep 19, 2022

View reviewed changes

joseph-robertson added 4 commits September 19, 2022 10:54

Clean up ft files.

d33bfd6

Improve ft and vt tests.

f4fa781

Typos.

26ae52a

Fix up ft test.

7cc8064

joseph-robertson and others added 3 commits September 19, 2022 16:44

Check node connections.

9a67761

Merge branch 'v22.2.0-IOFreeze' into new-watertoair-fields

f10eb5e

Merge remote-tracking branch 'refs/remotes/origin/v22.2.0-IOFreeze'

45b97a3

Conflicts: src/osversion/VersionTranslator.cpp src/osversion/test/VersionTranslator_GTest.cpp

jmarrec requested changes Sep 20, 2022

View reviewed changes

jmarrec reviewed Sep 20, 2022

View reviewed changes

Redo full test FT Tests for ZoneHVACWaterToAirHeatPump + Coils

d04a448

jmarrec merged commit 731a156 into v22.2.0-IOFreeze Sep 20, 2022

jmarrec deleted the new-watertoair-fields branch September 20, 2022 09:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

New Coil:*:WaterToAirHeatPump:EquationFit fields#4671

New Coil:*:WaterToAirHeatPump:EquationFit fields#4671
jmarrec merged 32 commits intov22.2.0-IOFreezefrom
new-watertoair-fields

joseph-robertson commented Sep 6, 2022 •

edited

Loading

Uh oh!

jmarrec left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmarrec Sep 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ci-commercialbuildings commented Sep 19, 2022 •

edited

Loading

Uh oh!

jmarrec left a comment

Uh oh!

jmarrec Sep 20, 2022

Uh oh!

jmarrec Sep 20, 2022

Uh oh!

jmarrec commented Sep 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		EXPECT_EQ(idf_fan.getString(Fan_OnOffFields::AirOutletNodeName).get(),
		idf_supHC.getString(Coil_Heating_ElectricFields::AirInletNodeName).get(),

Comments

Conversation

joseph-robertson commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

jmarrec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmarrec Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ci-commercialbuildings commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarrec left a comment

Choose a reason for hiding this comment

Uh oh!

jmarrec Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

jmarrec Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Sep 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joseph-robertson commented Sep 6, 2022 •

edited

Loading

ci-commercialbuildings commented Sep 19, 2022 •

edited

Loading