Skip to content

Comments

AirloopHVAC:UnitarySystem control type = SingleZoneVAV crashes with autosized water coils#7258

Merged
Myoldmopar merged 3 commits intodevelopfrom
7085-UnitarySystem-with-SingleZoneVAV-control-crashes
May 16, 2019
Merged

AirloopHVAC:UnitarySystem control type = SingleZoneVAV crashes with autosized water coils#7258
Myoldmopar merged 3 commits intodevelopfrom
7085-UnitarySystem-with-SingleZoneVAV-control-crashes

Conversation

@rraustad
Copy link
Collaborator

@rraustad rraustad commented Apr 8, 2019

Pull request overview

This fixes #7085 where autosized waters coils were not sizing correctly and caused a divide by 0 crash. The water coils do not have a capacity input field and capacity is used to calculate water volume flow rate. The result was water flow = 0 and the divide by 0 problem surfaced. It is also possible that the water coil could size to 0 flow and protection was added to ensure the model would run successfully.

Work Checklist

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

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
    • Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

  • Functional code review (it has to work!)
  • If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check
  • Unit Test(s)
  • C++ checks:
    • Argument types
    • If any virtual classes, ensure virtual destructor included, other things
  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults
    • Open windows IDF Editor with modified IDD to check for errors
    • If transition, add to input rules file for interfaces
    • If transition, add transition source
    • If transition, update idfs
  • If new idf included, locally check the err file and other outputs
  • Required documentation updates?
  • ExpandObjects changes?
  • If output changes, including tabular output structure, add to output rules file for interfaces

@rraustad
Copy link
Collaborator Author

rraustad commented Apr 8, 2019

Defect file: 7085-HVACTemplate-5ZoneUnitarySystem-SinglZoneVAV-exp-DXandChW.idf. Not sure why cooling doesn't respond same as heating. Although this is Chicago Ohare weather and maybe cooling air flow rarely needed to increase above minimum.

AirLoopHVAC:UnitarySystem,
  Sys 5 HW ChW System Unitary System,                      !- Name
  SingleZoneVAV,                                           !- Control Type
  SPACE5-1,                                                !- Controlling Zone or Thermostat Location
  None,                                                    !- Dehumidification Control Type
  ,                                                        !- Availability Schedule Name
  Sys 5 HW ChW System Mixed Air Outlet,                    !- Air Inlet Node Name
  Sys 5 HW ChW System Heating Coil Outlet,                 !- Air Outlet Node Name
  Fan:SystemModel,                                         !- Supply Fan Object Type
  Sys 5 HW ChW System Supply Fan,                          !- Supply Fan Name
  BlowThrough,                                             !- Fan Placement
  FanAvailSched,                                           !- Supply Air Fan Operating Mode Schedule Name
  Coil:Heating:Water,                                      !- Heating Coil Object Type
  Sys 5 HW ChW System Heating Coil,                        !- Heating Coil Name
  1.0,                                                     !- DX Heating Coil Sizing Ratio
  Coil:Cooling:Water,                                      !- Cooling Coil Object Type
  Sys 5 HW ChW System Cooling Coil,                        !- Cooling Coil Name

image

@rraustad
Copy link
Collaborator Author

rraustad commented Apr 8, 2019

@Myoldmopar looks like a CI problem with finding necessary files (e.g., ReadVars)

@Myoldmopar
Copy link
Member

Yes, CI is currently undergoing a major overhaul (vastly improved Decent CI codebase, new CI machine, updated E+ CI related stuff), so I'm expecting some speedbumps this week. Let me get the machines rolling out some results today and tomorrow and then we'll see where we stand. Thanks!

@nrel-bot
Copy link

@rraustad it has been 7 days since this pull request was last updated.

@nrel-bot
Copy link

@rraustad it has been 8 days since this pull request was last updated.

@nrel-bot
Copy link

nrel-bot commented May 4, 2019

@rraustad it has been 7 days since this pull request was last updated.

if (thisSys.MaxHeatCoilFluidFlow == DataSizing::AutoSize) {
thisSys.m_RequestAutoSize = true;
thisSys.m_DesignHeatingCapacity = DataSizing::AutoSize;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so just setting the heating capacity to autosize everywhere in this setup function, not just setting the autosize flag. Fine.

Copy link
Collaborator Author

@rraustad rraustad May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the intent was to set thisSys.m_DesignHeatingCapacity = DataSizing::AutoSize; only for coils that did not have a Capacity input, otherwise thisSys.m_DesignHeatingCapacity is initialized to =0 and there is a problem when sizing. The Coil:Heating:Water does have a capacity input field, although it's hard for water coils to know the design capacity for the alternate sizing method of UA and water flow rate (during GetInput). I wonder if this is why the diff's file has differences?

0.0001); // fan PLR at minimum speed
EXPECT_LT(DataLoopNode::Node(OutletNode).Temp, thisSys->DesignMaxOutletTemp); // outlet temperature does not exceed max limit

// test with 0 water flow rate to ensure divide by 0 does not happen (plant off, size = 0, etc.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, heating coil unit test for this condition. 👍

EXPECT_NEAR(thisSys->FanPartLoadRatio, 0.5117, 0.0001); // fan PLR above minimum speed
EXPECT_NEAR(DataLoopNode::Node(OutletNode).Temp, thisSys->DesignMinOutletTemp, 0.01); // outlet temperature modulated to meet max limit

// test with 0 water flow rate to ensure divide by 0 does not happen (plant off, size = 0, etc.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the cooling coil test. 👍

if (HeatingLoad) { // Function UnitarySystems::calcUnitarySystemToLoad, 4th and 5th arguments are CoolPLR and HeatPLR
// set the water flow ratio so water coil gets proper flow
SZVAVModel.HeatCoilWaterFlowRatio = maxCoilFluidFlow / SZVAVModel.MaxHeatCoilFluidFlow;
if (SZVAVModel.MaxHeatCoilFluidFlow > 0.0) SZVAVModel.HeatCoilWaterFlowRatio = maxCoilFluidFlow / SZVAVModel.MaxHeatCoilFluidFlow;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if these checks could be done in one location ahead of calling this function, but I see that each case has different evaluations, so unless things were significantly refactored (out of scope for this defect), I don't think it would be feasible here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I am considering condensing SZVAVModel.cc into one function and also making this type of change.

if (!coilActive) { // if the coil is schedule off or the plant cannot provide water
DataLoopNode::Node(coilFluidInletNode).MassFlowRate = 0.0;
if (coilLoopNum > 0)
if (coilLoopNum > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't really expecting this change, but it's reasonable to protect the node mass flow rate evaluation to only cases where the coil has a loop number.

@Myoldmopar
Copy link
Member

Myoldmopar commented May 14, 2019

Merged with develop locally and rebuilt. Verified that the defect file crashes with current develop branch. Verified the file runs to completion with this branch. Code changes look good. Weird CI issues are unrelated to your changes, but there are CI diffs. I'm investigating them now.

CHW LOOP CHILLER:Chiller Condenser Heat Transfer Rate [W] (Hourly) has absolute differences of nearly 200 W. That's significant. I'm wondering if it is because of the mass flow rate being set to zero differently here? Or maybe this branch was just behind develop when CI hit here? I don't recall any other branches causing that though. I am running the branch through my regression tool with the two diff files and we'll see.

Sure enough, there are still diffs. The chiller condenser rate really isn't off by that much as a percentage, but it's still pretty significant in absolute terms. Since I wasn't really expecting any diffs, this is too much:

image

I did try to revert the mass flow reset to zero changes you made in SZVAVModel.cc, and put the reset back outside the coil loop num block. Just for fun. I reran the diffs and they were still the same. I think I'm giving in for tonight. I'm going to push the branch back up now with develop merged in just to get shiny CI results. If we can resolve the diffs, this can go right in.

@rraustad
Copy link
Collaborator Author

rraustad commented May 14, 2019

Condenser heat transfer diff's may be due to a change in false loading that happens at minimum unloading ratio. The chiller capacity changed by -4%. In your figure, 4200W is well below 25% of design capacity so false loading is definitely a possibility. These diff's don't appear to happen at higher heat transfer rates (chiller PLRs). If the chiller now runs at a higher PLR (because design capacity is lower) then false loading would be reduced.

HVACTemplate-5ZoneUnitarySystem.expidf (the only one with condenser heat transfer rate report variable)

Chiller:Electric:EIR,
ChW Loop Chiller, !- Name
0.0, !- Minimum Part Load Ratio
0.25, !- Minimum Unloading Ratio

Output:Variable,*,Chiller False Load Heat Transfer Rate,hourly; !- HVAC Average [W] (could add to investigate)

    MinPartLoadRat = ElectricEIRChiller(EIRChillNum).MinPartLoadRat;
    MinUnloadRat = ElectricEIRChiller(EIRChillNum).MinUnloadRat;
    FRAC = 1.0;

    // Chiller cycles below minimum part load ratio, FRAC = amount of time chiller is ON during this time step
    if (PartLoadRat < MinPartLoadRat) FRAC = min(1.0, (PartLoadRat / MinPartLoadRat));
    PartLoadRat = max(PartLoadRat, MinUnloadRat);

    // calculate the load due to false loading on chiller over and above water side load
    ChillerFalseLoadRate = (AvailChillerCap * PartLoadRat * FRAC) - QEvaporator;

    QCondenser = Power * ElectricEIRChiller(EIRChillNum).CompPowerToCondenserFrac + QEvaporator + ChillerFalseLoadRate;

@Myoldmopar Myoldmopar added the Defect Includes code to repair a defect in EnergyPlus label May 16, 2019
@Myoldmopar
Copy link
Member

OK, I accept the justification. Merging this now.

@Myoldmopar Myoldmopar merged commit ff12a8d into develop May 16, 2019
@Myoldmopar Myoldmopar deleted the 7085-UnitarySystem-with-SingleZoneVAV-control-crashes branch May 16, 2019 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnitarySystem with SingleZoneVAV control crashes

7 participants