Fix #1675 - ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation) #4011

jmarrec · 2020-07-01T14:52:25Z

Pull request overview

Fixes WaterHeater:Mixed not forward translated when not on loop #1675
ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation)

Please read OpenStudio Pull Requests to better understand the OpenStudio Pull Request protocol.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

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

…dule is actually required for standalone operation

jmarrec · 2020-07-01T15:07:29Z

src/energyplus/ForwardTranslator.cpp

+  // If using a plantLoop, this will be translated by the PlantLoop. But WaterHeaters can also be used stand-alone, so always translate them
+  // We'll check in their FT if the "Peak Use Flow Rate" is actually initialized as it's an indication that the WH was
+  result.push_back(OS_WaterHeater_Mixed);
+  result.push_back(OS_WaterHeater_Stratified);


Always translate them...

jmarrec · 2020-07-01T15:11:11Z

src/energyplus/ForwardTranslator/ForwardTranslateWaterHeaterMixed.cpp

+  if (!modelObject.plantLoop() && !(modelObject.peakUseFlowRate() && modelObject.useFlowRateFractionSchedule()) ) {
+    LOG(Warn, modelObject.briefDescription() << " will not be translated as it not on a PlantLoop, and it does not have both a Peak Use Flow Rate "
+        "and a Use Flow Rate Fraction Schedule which is a required condition for stand-alone operation");
+    return boost::none;
+  }


BUT do not actually translate them to IDF if either:

On a PlantLoop

has both the peakUseFlowRate set (optional double) and the useFlowRateFractionSchedule (optional schedule).

Technically I think we could do without the useFlowRateFractionSchedule but whatever, it is desired here I think, as it'll avoid people setting the flow rate by mistake (in OS App for eg...) and ending up with "hidden" standalone heaters if they get orphaned from a plantloop.

E+ source shows that no severe will be issued if schedule not specified while standalone operation, and it basically amounts to using an Unity schedule:

https://github.com/NREL/EnergyPlus/blob/68c51b98f26f703a0f7b92ac7c30a18f8a526313/src/EnergyPlus/WaterThermalTanks.cc#L2577-L2586

https://github.com/NREL/EnergyPlus/blob/68c51b98f26f703a0f7b92ac7c30a18f8a526313/src/EnergyPlus/WaterThermalTanks.cc#L5969-L5987

@kbenne

typo

…tandalone

kbenne · 2020-08-10T15:23:09Z

@jmarrec I'm seeing this from the CI

/Users/jenkins/git/OpenStudioIncremental/develop/src/energyplus/Test/WaterHeaterMixed_GTest.cpp:64:19: note: in instantiation of template class 'boost::optional<openstudio::model::Schedule>' requested here
  EXPECT_FALSE(wh.useFlowRateFractionSchedule());

Which I think normally indicates a missing #include <Foo_Impl> Schedule_Impl in this case probably. Can you take a look?

…tandalone

jmarrec · 2020-08-10T18:29:46Z

@jmarrec I'm seeing this from the CI
/Users/jenkins/git/OpenStudioIncremental/develop/src/energyplus/Test/WaterHeaterMixed_GTest.cpp:64:19: note: in instantiation of template class 'boost::optional<openstudio::model::Schedule>' requested here
  EXPECT_FALSE(wh.useFlowRateFractionSchedule());
Which I think normally indicates a missing #include <Foo_Impl> Schedule_Impl in this case probably. Can you take a look?

Thanks @kbenne I think I just forgot to include Schedule.hpp. I pushed a fix, we'll see what CI reports.

…ater doesn't appear to be on a PlantLoop...

jmarrec · 2020-08-11T10:47:34Z

@kbenne I have a failing test and I cannot understand why.

From the Gtest itself, waterHeater.plantLoop() is perfectly true as it should.

OpenStudio/src/energyplus/Test/GeneratorMicroTurbine_GTest.cpp

Line 208 in 8bcd27d

EXPECT_TRUE(waterHeater.plantLoop());

Yet when I reach the FT, it is false?!?

OpenStudio/src/energyplus/ForwardTranslator/ForwardTranslateWaterHeaterMixed.cpp

Line 66 in 8bcd27d

    
           if (!(modelObject.plantLoop() || (modelObject.peakUseFlowRate() && modelObject.useFlowRateFractionSchedule()))) {

[ RUN      ] EnergyPlusFixture.ForwardTranslatorGeneratorMicroTurbine_ELCD_Orphan
[BOOST_ASSERT] <2> Assertion idf_component failed on line 106 of boost::optional<openstudio::IdfObject> openstudio::energyplus::ForwardTranslator::translatePlantEquipmentOperationHeatingLoad(openstudio::model::PlantEquipmentOperationHeatingLoad&) in file /home/julien/Software/Others/OpenStudio/src/energyplus/ForwardTranslator/ForwardTranslatePlantEquipmentOperationHeatingLoad.cpp.
openstudio_energyplus_tests: /home/julien/Software/Others/OpenStudio/src/energyplus/Test/../../model/../utilities/core/Assert.hpp:61: void boost::assertion_failed(const char*, const char*, const char*, long int): Assertion `false' failed.
[1/1] EnergyPlusFixture.ForwardTranslatorGeneratorMicroTurbine_ELCD_Orphan returned/aborted with exit code -6 (402 ms)
FAILED TESTS (1/1):
     402 ms: Products/openstudio_energyplus_tests EnergyPlusFixture.ForwardTranslatorGeneratorMicroTurbine_ELCD_Orphan
Built branch '1675_WaterHeater_Standalone' in 1min7s, tests ran in 0min6s

…o overide mchp::rmeove

ci-commercialbuildings · 2020-08-12T15:27:19Z

CI Results for 7c236bf:

Linux:
- OpenStudio-3.0.1-beta+7c236bf0fd-Linux.deb
- OpenStudio-3.0.1-beta+7c236bf0fd-Linux.tar.gz
Darwin:
- OpenStudio-3.0.1-beta+7c236bf0fd-Darwin.dmg
- OpenStudio-3.0.1-beta+7c236bf0fd-Darwin.tar.gz
Windows:

jmarrec · 2020-08-12T15:36:09Z

Linux and Mac are happy, the ECLD_Orphan test now passes with the fix. Thanks @kbenne for helping me find the needle in the haystack.

This is ready to drop

…ed to bump to 3.1.0

Add tests for #1675

a7b4789

jmarrec added component - HVAC component - IDF Translation Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Jul 1, 2020

jmarrec self-assigned this Jul 1, 2020

Adjust test: after looking at E+ src, the use flow rate fraction sche…

ba6c890

…dule is actually required for standalone operation

jmarrec requested a review from kbenne July 1, 2020 15:06

jmarrec commented Jul 1, 2020

View reviewed changes

Fix #1675

0b78791

typo

jmarrec force-pushed the 1675_WaterHeater_Standalone branch from 394bdf8 to 0b78791 Compare July 1, 2020 15:14

Merge remote-tracking branch 'origin/develop' into 1675_WaterHeater_S…

44231ba

…tandalone

jmarrec added 2 commits August 10, 2020 20:24

Merge remote-tracking branch 'origin/develop' into 1675_WaterHeater_S…

85399ac

…tandalone

Missing include for Gtest

0cc3cc2

jmarrec added 2 commits August 11, 2020 11:31

Fix build error

ed5ddd9

No clue why the test is failing.... Within the FT itself, the waterHe…

8bcd27d

…ater doesn't appear to be on a PlantLoop...

the mchp is removed when not on an ELCD at the begining of FT. Need t…

922c2ca

…o overide mchp::rmeove

jmarrec added a commit that referenced this pull request Aug 12, 2020

Prepare VT and VT Test, pending #4011 to avoid merge conflcits and ne…

0189b61

…ed to bump to 3.1.0

kbenne merged commit 94454c1 into develop Aug 14, 2020

kbenne deleted the 1675_WaterHeater_Standalone branch August 14, 2020 16:42

jmarrec mentioned this pull request Aug 27, 2020

refrigeration_system: Tank attached to Coil:WaterHeating:Desuperheater isn't connected to anything NREL/OpenStudio-resources#113

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1675 - ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation) #4011

Fix #1675 - ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation) #4011

jmarrec commented Jul 1, 2020

jmarrec Jul 1, 2020

jmarrec Jul 1, 2020

kbenne commented Aug 10, 2020 •

edited

Loading

jmarrec commented Aug 10, 2020

jmarrec commented Aug 11, 2020 •

edited

Loading

ci-commercialbuildings commented Aug 12, 2020 •

edited

Loading

jmarrec commented Aug 12, 2020

Fix #1675 - ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation) #4011

Fix #1675 - ForwardTranslate WaterHeater:Mixed when not on loop but has Peak Use Flow Rate (standalone operation) #4011

Conversation

jmarrec commented Jul 1, 2020

Pull request overview

Pull Request Author

Review Checklist

jmarrec Jul 1, 2020

Choose a reason for hiding this comment

jmarrec Jul 1, 2020

Choose a reason for hiding this comment

kbenne commented Aug 10, 2020 • edited Loading

jmarrec commented Aug 10, 2020

jmarrec commented Aug 11, 2020 • edited Loading

ci-commercialbuildings commented Aug 12, 2020 • edited Loading

jmarrec commented Aug 12, 2020

kbenne commented Aug 10, 2020 •

edited

Loading

jmarrec commented Aug 11, 2020 •

edited

Loading

ci-commercialbuildings commented Aug 12, 2020 •

edited

Loading