Correct system sizing when TU design sizing object is used

[rraustad]

Pull request overview

• Fixes Use of DesignSpecification:AirTerminal:Sizing gives incorrect results #10375
• The fractions used in DesignSpecification:TerminalUnit:Sizing where not applied correctly to system sizing.

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.

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• 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
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[rraustad]

This code is now using the same adjusted air flow to mass weight adjust system variables (e.g., RetTemp, etc.) that is summed here and used at line 5271 - 5274 to calculate the average. Also added new functions to apply TU sizing fractions to zone loads (i.e., applyTermUnitSizingCoolLoad and applyTermpUnitSizingHeatLoad).

[rraustad]

Added local to only do this math once.

[rraustad]

In this effort I am trying to understand the definition of HeatFlowSeqNoOA (and Cool*). In the defect/example file, HeatFlowSeqNoOA = 0 and I think it should be = HeatFlowSeq. These variables are handled differently between cooling and heating in updateZoneSizingEndZoneSizingCalc7 and I think that's where the problem lies.

[rraustad]

There were 2 changes in the last commit. The first initializes zone sizing Cool/HeatFlowSeqNoOA mass flow rate before any updates are made to the zone mass flow rate variable Cool/HeatFlowSeq (which can include an adjustment for minimum OA). The doc below shows a comparison of cooling and heating code in updateZoneSizingEndZoneSizingCalc7 along with the mods made here. This doc made it easier to figure things out. The second was to set a design day when NonCoincident system sizing is selected (because I was seeing blank design day strings in the eio). When a coincident peak is found the day/time is stored for reporting. For non-coincident sizing only the capacity and other information are stored for reporting (see end of EndSysSizingCalc). When non-coincident sizing is selected, there really is no design day/time except when all zones peak at the same time and no peak day/time is recorded. So now, when the code updates the non-coincident peak, the code also reports the peak day/time of the last non-zero air flow zone included in the non-coincident peak load sum (we can discuss this second change).

void updateZoneSizingEndZoneSizingCalc7_mod.zip

[rraustad]

When entering the outer IF block the peak sizing data is updated. However, the peak day/time is what was found for coincident sizing (or blank if there was no coincident peak). This change uses the peak day/time of the last zone in the loop as the reported day/time of non-coincident peak. For now this happens only when a coincident peak was not found (i.e., if (SensCoolPeakDD == 0 && .. )).

[rraustad]

Typo on line 6604 using HeatDDNum instead of CoolDDNum. When testing results for always printing this choice of non-coincident peak reporting (i.e., removing SensCoolPeakDD == 0) this became apparent. Once corrected, the system non-coincident cooling peak day/time matched the last zones peak day/time (previously showed 16:00 for system peak).

Zone Sizing Information, SPACE5-1, Cooling, 1168.36825, 1168.36825, 9.82782E-002, 0.13906, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 15:00:00
System Sizing Information, VAV SYS 1, Cooling, Sensible, 11210.35, 0.77212, 0.77212, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 15:00:00

[rraustad]

The Cool/HeatFlowSeqNoOA variables were not initialized except for inside the IF above this else. This code initializes these variables if the conditional fails for that IF.

[rraustad]

A discussion is needed on these conditionals for non-coincident results reporting. If either one of these are false non-coincident results are updated. This does not seem correct. In this case for heating, SysHeatCap can never be < 0 (it's capped at min = 0) so these non-coincident results are always reported. So then why need the conditional check at all? Is there something about AllOA sizing that needs consideration?

SysHeatCap = max(0.0, SysHeatCap);

// check to see if the noncoincident result is actually bigger than the coincident (for 100% outside air)
// why is this < 0.0 ? SysHeatCap cannot be < 0 ?? this code will always get executed
if (!(state.dataSize->FinalSysSizing(AirLoopNum).HeatOAOption == OAControl::AllOA &&
      SysHeatCap < 0.0)) { // HeatOAOption = Yes 100% OA
    state.dataSize->CalcSysSizing(AirLoopNum).HeatCap = SysHeatCap;

For cooling, non-coincident SysSensCoolCap could be 0 and in that case non-coincident result are updated only for CoolOAOption = MinOA. Shouldn't these cooling and heating non-coincident peak results always be reported if the user selected that choice? Even if the non-coincident peak capacity = 0 and coincident peak capacity > 0? or if non-coincident peak is less than coincident peak? And non-coincident sizing seems to look only at sensible peak. I am not sure if Sizing:System Type of Load to Size On = Total is allowed but is not included in this logic.

SysSensCoolCap = max(0.0, SysSensCoolCap);
SysTotCoolCap = max(0.0, SysTotCoolCap);

// But first check to see if the noncoincident result is actually bigger than the coincident (for 100% outside air)
if (!(state.dataSize->FinalSysSizing(AirLoopNum).CoolOAOption == OAControl::AllOA &&
      SysSensCoolCap <= 0.0)) { // CoolOAOption = Yes 100% OA
    state.dataSize->CalcSysSizing(AirLoopNum).SensCoolCap = SysSensCoolCap;

[rraustad]

Will save the index of the last TU scanned for this air loop. Since the zone peak will likely occur at different times, this will report one of the zones (the last one scanned) peak time.

[mjwitte]

When non-coincident sizing is selected, there really is no design day/time except when all zones peak at the same time and no peak day/time is recorded. So now, when the code updates the non-coincident peak, the code also reports the peak day/time of the last non-zero air flow zone included in the non-coincident peak load sum (we can discuss this second change).

As you say, for non-coincident sizing there is no design day/time, so why pick one to report, it should be left blank, no?

[rraustad]

@mjwitte I went back and forth on whether to report a non-coincident peak time. I kept thinking about the case where the zones peak at or near the same day and time and whether showing the user at least a peak time for one of the zones was useful. I landed on a non-blank report of the non-coincident of peak time. I would still need to add this description to the docs if in fact we choose to retain this mehod or use N/A instead.

[mjwitte]

@mjwitte I went back and forth on whether to report a non-coincident peak time. I kept thinking about the case where the zones peak at or near the same day and time and whether showing the user at least a peak time for one of the zones was useful. I landed on a non-blank report of the non-coincident of peak time. I would still need to add this description to the docs if in fact we choose to retain this mehod or use N/A instead.

The non-coincident peak date/time should be for the system load calc which should be at the system flow over the design days, no? So, the TU peak dates/times are not relevant, it should be a true peak for the system load. I guess I need to review the load calc portion for the non-coincident system sizing.

[Myoldmopar]

Normalizing the way things are done is a nice outcome here, although I will yield to you on some of the sizing specifics @rraustad. If you feel comfortable with it, I'm good with this, let me know if you plan to do more or need help from me. @mjwitte any other thoughts here?

[Myoldmopar]

There is a typo here in the argument name, but no need to commit just for that.

I do have another note for later: in absolutely tiny little functions like these, you can put them in the struct declaration itself. In this case, they are also const on the struct since they don't modify any struct member variables. So it would look like:

struct Whatever {
  //...
  Real64 applyTermUnitSizingCoolLoad(Real64 const coolLoad) const {
    return coolLoad * this->SpecDesSensCoolingFrac;
  }
  Real64 applyTermUnitSizingHeatLoad(Real64 const coolLoad) const {
    return heatLoad * this->SpecDesSensCoolingFrac;
  }
  // etc.
}

and nothing in the cc file. If the functions get more complicated, or do anything with a state variable, they should go in the cc, so there is definitely not anything you need to do here. Just a note.

[mjwitte]

@Myoldmopar there's still a question about what to write for peak day/time for non-coincident sizing. @rraustad Are you ready for this to go in if that detail is resolved, or does this need to bake longer?

[rraustad]

I could wrap this up if we can decide what to report for non-coincident peak time.

[rraustad]

If I reset the heating peak DD = 0 and HeatDesDay = "NA", to compare with what is reported now for cooling results, I get:

System Sizing Information, VAV SYS 1, Cooling, Sensible, 11210.35, 0.77212, 0.77212, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 15:00:00
System Sizing Information, VAV SYS 1, Heating, Sensible, 0.00, 0.51292, 0.51292, NA,  00:00:00
System Sizing Information, DOAS, Cooling, Sensible, 5636.75, 0.26412, 0.26412, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 15:00:00
System Sizing Information, DOAS, Heating, Sensible, 9206.40, 0.26412, 0.26412, NA,  00:00:00

So do we want to report NA? or the peak time for the last zone connected to this air loop? or other thoughts?

[rraustad]

I reverted the non-coincident peak time reporting and this is what the defect file result looks like. Now I remember why I added reporting for non-coincident peak time. I'll open a new issue so that these changes can get merged (#10402).

System Sizing Information, VAV SYS 1, Cooling, Sensible, 11210.35, 0.77212, 0.77212, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 16:00:00
System Sizing Information, VAV SYS 1, Heating, Sensible, 0.00, 0.51292, 0.51292, ,  00:00:00
System Sizing Information, DOAS, Cooling, Sensible, 5636.75, 0.26412, 0.26412, CHICAGO_IL_USA ANNUAL COOLING 1% DESIGN CONDITIONS DB/MCWB, 7/21 20:15:00
System Sizing Information, DOAS, Heating, Sensible, 9206.40, 0.26412, 0.26412, ,  00:00:00

[mjwitte]

@rraustad This looks good now. Thanks.

[Myoldmopar]

Thanks @rraustad and @mjwitte, I pulled in develop since I just merged a PR, and everything still builds and passes happily. The diffs are minimized down to system sizing type of changes as expected here. I'll merge this in now.