Fix HVAC Sizing Summary Thermostat Setpoint Temperature at Peak Load when no peak heating load

[matthew-larson]

Pull request overview

• Fixes HVAC Sizing Summary Thermostat Setpoint Temperature at Peak Load outputs 0C when no peak load #8688
• This pull request sets the Thermostat Setpoint Temperature at Peak Load to the ZoneThermostatSetPointLo variable if there is no peak heating load. Originally, if HeatLoad equals 0, HeatTstatTemp never gets set, so the default value of 0 used and reported to the Zone Sensible Heating report. I added a line to set the HeatTstatTemp to the ZoneThermostatSetPointLo variable if HeatLoad equals 0 so as to override the default value and provide a more reasonable value to the Zone Sensible Heating report.

The alternative is to set the "... Peak Load" values to blank, similar to the Date/Time Of Peak column in the report.

Note: Need to commit unit test

Before:

After:

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

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

[rraustad]

I have a question. If TempZoneThermostatSetPoint > 0, and CoolTstatTemp = (is set equal to) TempZoneThermostatSetPoint (line 669 and 670), then why would HeatTstatTemp be set = to ZoneThermostatSetPointLo ? What if ZoneThermostatSetPointLo = 0 ? It hasn't been tested for being > 0. Why not set it to TempZoneThermostatSetPoint ? which is already tested for > 0. Also, you only made 1 change here, that gives me pause given the multiple possible load results. If (heatingload > 0), else take care of everything else.

I realize you are following this existing code. Where ZoneThermostatSetPointLo is used if TempZoneThermostatSetPoint = 0. But here, TempZoneThermostatSetPoint > 0.

    if (state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).HeatLoad > 0.0) {
        state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).HeatZoneRetTemp = RetTemp;
        if (state.dataHeatBalFanSys->TempZoneThermostatSetPoint(ActualZoneNum) > 0.0) {
            state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).HeatTstatTemp =
                state.dataHeatBalFanSys->TempZoneThermostatSetPoint(ActualZoneNum);
        } else {
            state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).HeatTstatTemp =
                state.dataHeatBalFanSys->ZoneThermostatSetPointLo(ActualZoneNum);
        }
    } else {
        state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).CoolZoneRetTemp = RetTemp;
        if (state.dataHeatBalFanSys->TempZoneThermostatSetPoint(ActualZoneNum) > 0.0) {
            state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).CoolTstatTemp =
                state.dataHeatBalFanSys->TempZoneThermostatSetPoint(ActualZoneNum);
            state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).HeatTstatTemp =
                state.dataHeatBalFanSys->ZoneThermostatSetPointLo(ActualZoneNum);   <<<<<<<
        } else {
            state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum).CoolTstatTemp =
                state.dataHeatBalFanSys->ZoneThermostatSetPointHi(ActualZoneNum);
        }
    }

This is my first pass at this and was rather quick. Just pointing out what I saw on this pass.

[matthew-larson]

Thanks for doing a quick initial pass on this. I see your reasoning with TempZoneThermostatSetPoint and there are some additional situations I need to test for. I did try using this variable and the Thermostat Setpoint Temperature at Peak Load results in a somewhat random value that doesn't match any setpoint values.

My thinking was adding these lines in the else statement, it basically provides a default for this value that matches the heating setpoint value since it is covering when HeatLoad or CoolLoad is equal to 0. Maybe there is a better way to achieve this however as I was trying to following existing code. Is it better to have a separate if statement for if HeatTstatTemp is equal to 0? This does seem to achieve the same thing. I can discuss with Neal in more detail this week as well.

[matthew-larson]

@rraustad Thinking about this more, is it more appropriate to have the columns related to the Peak Load be blank if there is no Date/Time Of Peak? There really is no value that is correct in this situation when there is no peak load. I feel this could be accomplished with a simple check if there is no heating or cooling peak load but wanted to get your thoughts on this first.

[rraustad]

Not sure about that. Should probably do the same thing Indoor Temperature at Peak Load is doing Just because there is no load does not mean there is no thermostat.

[nealkruis]

@rraustad If there are no loads (and therefore no peak loads), maybe it would make sense to report the values corresponding to the min difference between the indoor temperature and the set point temperature. That way, the report would illustrate how far away from the setpoint the zone actually gets when there is no load.

[rraustad]

That sounds like a good solution. For a Tstat temp of 70F, the indoor temp might be 71.2F. That would definitely show why there is no zone load.

[matthew-larson]

That makes sense and provides the most value to the user. I'll work on going this route.

[rraustad]

So then it would look like this.

[nealkruis]

Right, except we might as well add the date/time of the min temperature difference as well (and all the other coincident values in the other columns).

[Myoldmopar]

Windows CI got a little sick the last couple days (Covid negative, just allergies I promise), so ignore that red CI result for now. If I'm understanding it correctly though, we are expecting this to get to no diffs? Or at least no big diffs? We will be starting moving toward code freeze early-mid week, so if this feels like it's going to be too much, then go ahead and change the milestone. Otherwise I'll just be watching for commits to come in. Thanks @matthew-larson and @rraustad

[matthew-larson]

There will be some diffs for the zone sizing information to include the date/time and change in Thermostat Setpoint Temperature at Peak Load but still seeing some diffs in the system sizing that shouldn't be there, hoping to resolve this today.

[Myoldmopar]

@matthew-larson I can run the latest commit to check for regressions locally if you would like. Let me know if you think it's ready for that or not.

[Myoldmopar]

I did go ahead and kick off a build/test of this, but didn't make it to regressions. Even when just running unit tests I encountered a few that failed with:

685: ...for Sizing Period: #2 MIAMI INTL AP ANN CLG .4% CONDNS DB=>MWB
685: **FATAL:Please send your input file to the EnergyPlus support/development team for further investigation.
685: EnergyPlus Run Time=00hr 00min  0.14sec

That error message only occurs in a few random spots in the code (and obviously the message could be cleaned up, but that's a different day). Anyway, it feels like this is still not ready. If it is unlikely to get addressed over the weekend, then this is likely to get punted to after release.

[matthew-larson]

I found that too, working through this right now. I'll do what I can to try to get it resolved but if it looks like too much too quickly, I'll push it back.

[matthew-larson]

I was able to resolve the issue with the unit tests. I'm expecting a handful of diffs with the EIO and HTML files for the Zone Sizing Information adding the date/time and the Component Sizing Information for any system coils, but will check those in detail once the CI does it's thing and provide justification.

[matthew-larson]

Broke out parameters for when there is a cooling load and when there is no heating or cooling load. Always defining the ZoneTemp and ZoeHumRat temp to determine min/max temperature to output minimum temperature at no heating load condition and maximum temperature at no cooling load condition.

[rraustad]

Yes, this does give better control of which vars get set based on load, or no load. HeatZoneTemp and CoolZoneTemp are set all the time now, that's what we are doing over in a 3rd party spin off. Nice!

[Myoldmopar]

👍

[Myoldmopar]

Yeah, unit tests do run a bit better now 👍 Regressions are currently running and I'll report back shortly.

[Myoldmopar]

My results, trying to aggregate on the fly so the numbers might be off by a couple:

• 23 files with big math diffs
• 6 files with small math diffs
• ~40 table diffs
• 50 text diffs, mostly EIO, but some ERR and one EDD

[matthew-larson]

Found a small issue that was causing some of the model diffs that shouldn't be there (5ZoneSwimmingPool.idf for example). Just pulled in develop and pushed up the latest, should get rid of some additional diffs without hopefully causing more with any models.

[matthew-larson]

These latest diffs look better as they are only for models that contain a zone(s) that don't contain a heating or cooling load. I can go into more detail later tonight/tomorrow as well.

[Myoldmopar]

I am happy the diffs count has gone down. This is definitely looking better. I ran tests locally and everything seems fine. I can look back over the changes, but would love if anyone else wants to chime in. @rraustad it looks like above you are pleased with some of the changes as that is what you are already doing in the fork. If I don't hear otherwise then I'll look this over in a bit and see if I can drop it in.

[Myoldmopar]

👍

[Myoldmopar]

👍

[matthew-larson]

Thanks @Myoldmopar!