Allow multistage electric coil to be supplemental coil of UnitarySystem by yzhou601 · Pull Request #9414 · NatLabRockies/EnergyPlus

yzhou601 · 2022-05-05T21:41:34Z

Pull request overview

Addresses the first limitation described in Multi-stage electric backup coil modeling #9357
Add Coil:Heating:Electric:MultiStage as one of supplemental coil options for AirLoopHVAC:UnitarySystem
Corrects set point controls for Multispeed coils and Fixes UnitarySystem multi-speed supplemental heating coils do not control correctly when using Setpoint control #9483

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

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

Reviewer

This will not be exhaustively relevant to every PR.

…elop

…ow_multistage_backup_coil

src/EnergyPlus/HeatingCoils.cc

src/EnergyPlus/UnitarySystem.cc

…ow_multistage_backup_coil

rraustad · 2022-05-19T02:36:45Z

src/EnergyPlus/UnitarySystem.cc

+
+        HeatingCoils::CalcMultiStageElectricHeatingCoil(state, CoilIndex, SpeedRatio, CycRatio, SpeedNum, FanOpMode, QActual);
+
+        Residuum = Par[2] - QActual;


Usually these RootSolver functions return a fraction of load met, e.g., (Par[2] - QActual)/Par[2]. If the tolerance is 0.001 then that means with this math the RootSolver function will iterate to within 0.001 W. That's OK for heating coils most of the time because heating coils usually return the exact load request.

rraustad · 2022-05-19T02:50:26Z

Those diffs in the html show a change in ventilation. That probably means a change in supply air flow rate. You can compare SA and OA flow rate between this branch and develop and see if those show a change in operation.

rraustad · 2022-05-19T05:09:49Z

src/EnergyPlus/UnitarySystem.cc

-            SuppPLR = this->m_SuppHeatPartLoadFrac;
            // only need to test for high supply air temp if supplemental coil is operating
-            if (SuppPLR > 0.0) {
+            if (SupHeaterLoad > 0.0 && this->m_DesignSuppHeatingCapacity > 0.0) {


This change might be why there are diffs. I would revert this logic unless there is good reason for it. The cooling and heating coil are modeled first to determine the air flow rate and PLRs. If after that the supp heater is needed, then SuppPLR will be > 0 and the supp heater will turn on. Also, if (... && this->m_DesignSuppHeatingCapacity > 0.0), of course it is.

Changes to this part are needed to allow multistage backup coils, but I will try reverting the logic to be as close to previous one as possible for other cases.

You could use something like this (here and other places) just to get it to work and we can figure out better logic later. There should be no diffs from adding a coil that doesn't exist yet.

if (this->m_SuppCoilExists) { if( stagedheatingcoil) { insert your logic } else { existing code } }

src/EnergyPlus/HeatingCoils.cc


-        auto &HeatingCoil(state.dataHeatingCoils->HeatingCoil);
+        auto &heatingCoil = state.dataHeatingCoils->HeatingCoil(CoilNum);


rraustad · 2022-05-19T11:41:52Z

src/EnergyPlus/HeatingCoils.cc

-                HeatingCoil(CoilNum).HeatingCoilLoad = MSHPMassFlowRateHigh * (HSFullLoadOutAirEnth - InletAirEnthalpy) * SpeedRatio +
-                                                       MSHPMassFlowRateLow * (LSFullLoadOutAirEnth - InletAirEnthalpy) * (1.0 - SpeedRatio);
+                ElecHeatingCoilPower = heatingCoil.ElecUseLoad;
+                heatingCoil.HeatingCoilLoad = TotCapHS * SpeedRatio + TotCapLS * (1.0 - SpeedRatio);


I am trying to find a reason for the diff in _MultiSpeedACElecFurnace.idf. That example file uses AirLoopHVAC:UnitaryHeatPump:AirToAir:MultiSpeed (no changes) with Coil:Heating:Electric:MultiStage so it must be some change in HeatingCoils that changed that answer. It looks like these are the same but I wonder if there is a rounding difference that causes the diff. Have you plotted SA and OA air flow rate and heating coil rate to see if they are very similar?

Note that I have quickly fixed two lines of codes: https://github.com/NREL/EnergyPlus/pull/9414/files/78461457cb705a42f132d0c8037c1f66401dbba8#diff-a4469a58b3b57ae5c4916cb9efb12f0c6ef79224175d09c796a7f6c2e1d3ec33R9039
and:
https://github.com/NREL/EnergyPlus/pull/9414/files/78461457cb705a42f132d0c8037c1f66401dbba8#diff-a4469a58b3b57ae5c4916cb9efb12f0c6ef79224175d09c796a7f6c2e1d3ec33R15991

These two lines looked pretty clear to be bugs, but they might cause diffs. I will revert them and see whether that's the case.

If you find actual bugs during development you should open a new issue and put those few lines in a seperate PR and then get that PR merged. Then when you pull develop those diffs, related to that necessary change, will be gone. And the defect documented with an issue and PR specific to that bug.

rraustad · 2022-05-19T11:49:30Z

src/EnergyPlus/HeatingCoils.cc

-                                              OpMode); // Autodesk:OPTIONAL SpeedRatio, PartLoadRatio, StageNum used without PRESENT check
-        } else if (state.dataHeatingCoils->HeatingCoil(CoilNum).HCoilType_Num == Coil_HeatingGasOrOtherFuel) {
+                                              OpMode,
+                                              QCoilActual2); // Autodesk:OPTIONAL SpeedRatio, PartLoadRatio, StageNum used without PRESENT check


Regarding diffs, the other change I see is you are now passing &QCoilActual so I am wondering if this fixed something now that the Calc function has more information.

…ow_multistage_backup_coil

yzhou601 · 2022-06-10T22:00:31Z

tst/EnergyPlus/unit/UnitarySystem.unit.cc

+                      latOut);
+    // check that heating coil air outlet node is at maximum supply air temperature
+    //
+    // This is not passing because main heating coil is not limited by Maximum Supply Temperature correctly.


@rraustad Please see this. I have fixed the maximum supply air issue on supp coil side, but the main coil is still not correct. It is because the recalculated loads are not used in calcUnitaryHeatingSystem so the PLR is not recalculated.
I changed the unitary system parent object in the newly added test file, so the existing test file is possible to operate differently.

Yeah, that's OK. The main heating coil does not limit coil outlet air temp. And if sized properly should not present high outlet temps. This is the reason I suggested autosizing this file since the Sizing:Zone object has a supply air temp input. With these final changes I think this is ready to merge but I will make a final pass to make sure.

rraustad · 2022-06-10T23:23:06Z

This is working well, the supp heating coil can limit max outlet temp. And this is so much better now, the main heating coil outlet temp is around 35 C, that seems very reasonable.

AirLoopHVAC:UnitarySystem,
  45.0,                    !- Maximum Supply Air Temperature {C}

rraustad · 2022-06-11T00:30:44Z

src/EnergyPlus/UnitarySystem.cc

+                if (this->m_NumOfSpeedSuppHeating > 0) {
+                    this->calcMultiStageSuppCoilStageByLoad(state, MaxHeatCoilLoad, FirstHVACIteration);
+                }
                this->calcUnitarySuppHeatingSystem(state, FirstHVACIteration, SuppPLR, MaxHeatCoilLoad);


This is the call that bothers me. To call a new supp heater model, and then call the calcUnitarySuppHeatingSystem function to simulate that coil model is redundant. It seems the call to the new coil model can be incorporated into existing calls. This can be resolved later since UnitarySystem needs a major overhaul to clean things up. This is working so well that I hesitate to muck with it now.

…ow_multistage_backup_coil

rraustad · 2022-06-12T00:23:24Z

@yzhou601 this does look good for load based control. I changed the control for the new example file to Setpoint and found an issue. I haven't locked in on an answer yet (#9482 is in play) but RootSolver is calling the DX coil model for the supp MS heating coil. This should be easy to resolve. The new example file is SingleFamilyHouse_TwoSpeed_MultiStageElectricSuppCoil_SPControl,

rraustad · 2022-06-12T16:05:20Z

Warning is addressed in #9482. The problem with set point control is here where the heating coil is called instead of the supp heater.

Par[9] = true; // is supplemental coil
if (this->m_SuppHeatingSpeedNum > 1) {
    Par[4] = CycRatio;
    General::SolveRoot(state, Acc, MaxIte, SolFla, SpeedRatio, this->heatingCoilVarSpeedResidual, 0.0, 1.0, Par);
    this->m_SuppHeatingCycRatio = CycRatio;


Real64 UnitarySys::heatingCoilVarSpeedResidual(EnergyPlusData &state,
                                               Real64 const SpeedRatio,       // compressor speed ratio (1.0 is max, 0.0 is min)
                                               std::vector<Real64> const &Par // par(1) = DX coil number

    switch (thisSys.m_HeatingCoilType_Num) {

and here

Real64 UnitarySys::heatingCoilVarSpeedCycResidual(EnergyPlusData &state,
                                                  Real64 const CycRatio,         // compressor cycling ratio (1.0 is continuous, 0.0 is off)
                                                  std::vector<Real64> const &Par // par(1) = DX coil number

    switch (thisSys.m_HeatingCoilType_Num) {

rraustad · 2022-06-12T20:22:10Z

src/EnergyPlus/UnitarySystem.cc

            }
            LatLoad = 0.0;
-            this->m_FanOpMode = 1; // why is this here?
+            // this->m_FanOpMode = 1; // why is this here?


Deleted in #9482.

rraustad · 2022-06-13T12:25:19Z

The new load based example file:

rraustad · 2022-06-13T12:26:58Z

The new set point based example file:

rraustad · 2022-06-13T13:01:40Z

Added issue #9484 to address control issue with Coil:Cooling:DX:MultiSpeed. See top left figure above for new example file using set point control.

rraustad · 2022-06-13T13:08:41Z

@yzhou601 I think this is ready to merge. Waiting on review and merge of #9482 and any comments you may have on changes made over the weekend.

…ow_multistage_backup_coil

yzhou601 · 2022-06-13T16:14:40Z

@rraustad Thanks for fixing a few issues, these commits look good to me. Good catch!

rraustad · 2022-06-13T16:56:38Z

Unit tests run clean locally. Warnings in err file are described in #9484 and should be cleaned up with that effort. Expect CI to be all green.

rraustad · 2022-06-13T19:55:34Z

This all looks good, CI is green. There is a little time before merging for anyone to look over this.

Myoldmopar

I left a few comments in here. In general, these changes are exactly what I would expect for this new capability. The two things I mentioned that could be address are not related to algorithmic changes:

In format statements we shouldn't create std::string objects as args unnecessarily
Your new residual functions mimic the existing residual functions, and they are a bit more verbose than necessary.

I don't actually think that either of these necessitate additional commits on their own. If any other commits are needed, please address these as part of that.

idd/Energy+.idd.in

src/EnergyPlus/HeatingCoils.cc

src/EnergyPlus/UnitarySystem.cc

yzhou601 · 2022-06-13T23:59:53Z

Thanks @Myoldmopar , pushed a cleanup commit that addressed your comments.

rraustad · 2022-06-14T01:53:16Z

This branch is 25 commits behind develop and diff is from SurfLWR example file. No need to run another CI cycle.

yzhou601

@rraustad This PR was merged but these two lines of changes were reverted. I want to point them out to make sure you realize it.

yzhou601 · 2022-07-07T20:02:08Z

src/EnergyPlus/UnitarySystem.cc

                        PartLoadRatio = HeatPLR;
                    } else if (state.dataGlobal->DoCoilDirectSolutions && state.dataUnitarySystems->HeatingLoad &&
-                               this->m_HeatingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedHeating) {
+                               this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedHeating) {


@rraustad Hi, Richard, just want to point out that in this commit I reverted two lines of codes to eliminate changes unrelated to my added feature. But these two lines look very suspicious, could you double check if these are bugs?

Yes, this looks wrong.

Actually, this one was here before this branch.

yzhou601 · 2022-07-07T20:02:17Z

src/EnergyPlus/UnitarySystem.cc

            FanOpMode = int(Par[6]);

-            HeatingCoils::CalcMultiStageGasHeatingCoil(state, CoilIndex, SpeedRatio, CycRatio, SpeedNum, FanOpMode);
+            HeatingCoils::CalcMultiStageElectricHeatingCoil(state, CoilIndex, SpeedRatio, CycRatio, SpeedNum, FanOpMode, QActual);


This line as well

Yes, this also looks wrong.

case DataHVACGlobals::Coil_HeatingGas_MultiStage: { CycRatio = Par[4]; SpeedNum = int(Par[5]); FanOpMode = int(Par[6]); HeatingCoils::CalcMultiStageElectricHeatingCoil(state, CoilIndex, SpeedRatio, CycRatio, SpeedNum, FanOpMode, QActual, SuppHeat);

I agree, these bugs were here before this branch.

This one appears to have been correct and is now incorrect.

They are both bugs at this point.

yzhou601 added 9 commits August 18, 2021 15:28

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

8120bde

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

0dee2db

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

de136cb

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

5460e19

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

e1e2a35

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

8d52fa7

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

dcfbd65

…elop

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into dev…

c70e7e7

…elop

first cut

60da154

yzhou601 self-assigned this May 5, 2022

yzhou601 added Defect Includes code to repair a defect in EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels May 5, 2022

yzhou601 added 3 commits May 5, 2022 15:47

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into all…

e07170c

…ow_multistage_backup_coil

extra qualification error fix

aa45211

fix format

cf054ce

amirroth reviewed May 9, 2022

View reviewed changes

src/EnergyPlus/HeatingCoils.cc Outdated Show resolved Hide resolved

amirroth reviewed May 9, 2022

View reviewed changes

src/EnergyPlus/UnitarySystem.cc Show resolved Hide resolved

yzhou601 added 5 commits May 11, 2022 15:41

address comments, and a few other minor cleanups

63dd344

Merge branch 'develop' and a bugfix

258ba11

address comments

f571d03

fix

4022018

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into all…

7846145

…ow_multistage_backup_coil

rraustad reviewed May 19, 2022

View reviewed changes

src/EnergyPlus/HeatingCoils.cc

auto &HeatingCoil(state.dataHeatingCoils->HeatingCoil);

auto &heatingCoil = state.dataHeatingCoils->HeatingCoil(CoilNum);

This comment was marked as off-topic.

Sign in to view

rraustad reviewed May 19, 2022

View reviewed changes

yzhou601 added 2 commits May 25, 2022 19:41

revert some changes, check ci diffs

d63eb42

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into all…

07f2a6c

…ow_multistage_backup_coil

yzhou601 commented Jun 10, 2022

View reviewed changes

rraustad reviewed Jun 11, 2022

View reviewed changes

rraustad added 2 commits June 11, 2022 20:13

Move new MS heating coil call to supp heater manager. Add new test file.

b40b1e5

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into all…

aa82a89

…ow_multistage_backup_coil

Correct supp heater set point control

2d6c9f8

rraustad reviewed Jun 12, 2022

View reviewed changes

rraustad added 2 commits June 12, 2022 16:28

Add new example file

cfb861a

Correct outlet temp used in SolveRoot function

7f4ee07

rraustad added 2 commits June 13, 2022 11:42

delete stray line of code

2305e6e

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into all…

bf22dbd

…ow_multistage_backup_coil

rraustad marked this pull request as ready for review June 13, 2022 16:19

Myoldmopar approved these changes Jun 13, 2022

View reviewed changes

cleanups

518500f

rraustad merged commit 2c94536 into develop Jun 14, 2022

rraustad deleted the allow_multistage_backup_coil branch June 14, 2022 10:42

yzhou601 commented Jul 7, 2022

View reviewed changes

yzhou601 mentioned this pull request Jul 8, 2022

Corrects Coil:Heating:Gas:MultiStage to call the gas coil model instead of electric coil model #9525

Merged

20 tasks

yzhou601 mentioned this pull request Aug 10, 2022

Wrap E+ 22.2 new supplemental heating coil type for AirLoopHVAC:UnitarySystem object NatLabRockies/OpenStudio#4646

Closed


		HeatingCoils::CalcMultiStageElectricHeatingCoil(state, CoilIndex, SpeedRatio, CycRatio, SpeedNum, FanOpMode, QActual);

		Residuum = Par[2] - QActual;


		auto &HeatingCoil(state.dataHeatingCoils->HeatingCoil);
		auto &heatingCoil = state.dataHeatingCoils->HeatingCoil(CoilNum);

Comments

Conversation

yzhou601 commented May 5, 2022 • edited by rraustad Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Reviewer

Uh oh!

Uh oh!

Uh oh!

rraustad May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad commented May 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad commented Jun 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rraustad commented Jun 12, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad commented Jun 13, 2022

Uh oh!

rraustad commented Jun 13, 2022

Uh oh!

rraustad commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rraustad commented Jun 13, 2022

Uh oh!

yzhou601 commented Jun 13, 2022

Uh oh!

rraustad commented Jun 13, 2022

Uh oh!

rraustad commented Jun 13, 2022

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yzhou601 commented Jun 13, 2022

Uh oh!

rraustad commented Jun 14, 2022

Uh oh!

yzhou601 commented May 5, 2022 •

edited by rraustad

Loading

rraustad May 19, 2022 •

edited

Loading

rraustad commented Jun 10, 2022 •

edited

Loading

rraustad commented Jun 12, 2022 •

edited

Loading

rraustad commented Jun 13, 2022 •

edited

Loading