Correction of Mechanical Ventilation Controller Index Problem #9580
Conversation
Fix for the problem. Also, a new IDF was added to EnergyPlusDevSupport under 9558 that shows the differences before and after the fix. Mostly happens during the July run period.
Added the unit test of the mechanical ventilation fix for ACH control. Also, dropped in a couple of other random IDD and doc fixes from a previous release that missed the cut-off deadline.
Changes to IDD required change to input file. This should take care of the problem.
RKStrand
added
Defect
Forgot to clang forat this file. Not sure why it was so picky given that I just pasted in curZone but ok.
| ZoneOAACH = curZone.Multiplier * curZone.ListMultiplier * | ||
| (this->ZoneOAACHRate(ZoneIndex) * state.dataHeatBal->Zone(ZoneIndex).Volume) * curZoneOASchValue / 3600.0; | ||
| ZoneOAACH = | ||
| curZone.Multiplier * curZone.ListMultiplier * (this->ZoneOAACHRate(ZoneIndex) * curZone.Volume) * curZoneOASchValue / 3600.0; |
There was a problem hiding this comment.
Got it, so yeah, curZone is based on ZoneNum, ZoneIndex should not have been used here. Works for me.
| @@ -4120,7 +4120,7 @@ \subsubsection{Inputs}\label{inputs-38} | |||
|
|
|||
| \paragraph{Field: Tube Spacing}\label{field-tube-spacing} | |||
|
|
|||
| This field defines the distance between adjacent hydronic tubes spaced in the direction perpendicular to the main direction of heat transfer. The value for this parameter must be greater than 0.01m (or a tube spacing of 1 cm) and less than 1.0m (or a tube spacing of 1m). Note that this parameter is only used for two-dimensional solutions (see previous field) or when the user requests that the tube length for a hydronic radiant system ( \textbf{\hyperref[zonehvaclowtemperatureradiantvariableflow]{variable flow}} or \textbf{\hyperref[zonehvaclowtemperatureradiantconstantflow]{constant flow}}) be autosized. In the case of autosizing the tube length, this parameter is used along with the dimensions of the surface to approximate the tube length. | |||
| This field defines the distance between adjacent hydronic tubes spaced in the direction perpendicular to the main direction of heat transfer. The value for this parameter must be greater than or equal to 0.01m (or a tube spacing of 1 cm) and less than or equal to 1.0m (or a tube spacing of 1m). Note that this parameter is only used for two-dimensional solutions (see previous field) or when the user requests that the tube length for a hydronic radiant system ( \textbf{\hyperref[zonehvaclowtemperatureradiantvariableflow]{variable flow}} or \textbf{\hyperref[zonehvaclowtemperatureradiantconstantflow]{constant flow}}) be autosized. In the case of autosizing the tube length, this parameter is used along with the dimensions of the surface to approximate the tube length. | |||
There was a problem hiding this comment.
I don't see from the PR description anything about the tube size issue. Is that just something you identified while fixing the zone index issue and added it here?
| state->dataMixedAir->VentilationMechanical(1).SysDesOA = ExpectedOAMassFlow; // Set design outdoor air flow rate | ||
| state->dataSize->CurSysNum = 1; // Only one system in this instance | ||
| state->dataEnvrn->StdRhoAir = 1; // Standard air density assumed to be 1 kg/m3 (simplification) | ||
| state->dataMixedAir->VentilationMechanical(1).CalcMechVentController(*state, SysMassFlow, OAMassFlow); |
|
@Myoldmopar sorry, should have explained better. I combined in some IDD items that missed a previous release. They had nothing to do with this particular defect but I was trying to avoid having extra PRs. Is it okay to do this or do I need to create a separate branch to do this. |
Yes, this is perfectly OK, please combine away. Just make sure the PR description has enough detail so that I know what to expect with the changes being made. 👍 |
| @@ -90281,10 +90283,10 @@ ElectricLoadCenter:Storage:Simple, | |||
| \minimum 0.0 | |||
| N2, \field Nominal Energetic Efficiency for Charging | |||
| \maximum 1.0 | |||
| \minimum 0.0 | |||
| \minimum 0.001 | |||
There was a problem hiding this comment.
@Myoldmopar Well, the code that now exists uses 0.001 as a hard limit for both charging and discharging. I guess we could eliminate that logic now with the change to the IDD. If I remember correctly, my thinking was that it is going to divide by the number and I wanted to make sure we avoided the problem of having huge electric loads (an input file set this parameter to zero which ended up being a very tiny number and a massive electric load. I guess I'm okay with it like this, but I'll wait to hear if @mjwitte has a strong opinion.
There was a problem hiding this comment.
No opinion, really. Anything below 0.1 seems pathetic anyway, so I would hope nobody actually models anything this low.
| @@ -182,7 +182,7 @@ | |||
| 1, !- Thermal Source Present After Layer Number | |||
| 1, !- Temperature Calculation Requested After Layer Number | |||
| 1, !- Dimensions for the CTF Calculation | |||
| 0.0, !- Tube Spacing {m} | |||
| 0.1, !- Tube Spacing {m} | |||
|
Alright, with that resolved, let's merge this in. Thanks @RKStrand |
Myoldmopar
deleted the
9558IncorrectIndexMechanicalVentilationController
branch
Pull request overview
There was an improper index used for a calculation in the mechanical ventilation controller that would grab the wrong zone volume. This has been corrected and a unit test to verify the fix has been created. A few minor IDD, input file, and doc (IO Ref) fixes from check-ins in a previous release that missed the IDD freeze were tacked onto this work. No differences in the output as expected because there were no input files in the test suite where this problem was triggered.
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.