Skip to content

ZoneBaseboard:OutdoorTemperatureControlled autosizing is missing#11579

Open
joseph-robertson wants to merge 31 commits into
developfrom
zone-baseboard-autosizing
Open

ZoneBaseboard:OutdoorTemperatureControlled autosizing is missing#11579
joseph-robertson wants to merge 31 commits into
developfrom
zone-baseboard-autosizing

Conversation

@joseph-robertson
Copy link
Copy Markdown
Collaborator

@joseph-robertson joseph-robertson commented May 6, 2026

Pull request overview

5ZoneEndUses.idf:

image

Description of the purpose of this PR

Pull Request Author

  • 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
  • If adding/removing any output files (e.g., eplustbl.*)
    • Update ..\scripts\Epl-run.bat
    • Update ..\scripts\RunEPlus.bat
    • Update ..\src\EPLaunch\ MainModule.bas, epl-ui.frm, and epl.vbp (VersionComments)
    • Update ...github\workflows\energyplus.py

Reviewer

  • 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

@joseph-robertson joseph-robertson self-assigned this May 6, 2026
@joseph-robertson joseph-robertson 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 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 1993c6e

Regression Summary
  • Audit: 3
  • EIO: 3
  • Table Big Diffs: 3
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • ZSZ Big Diffs: 1
  • Table String Diffs: 1

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

⚠️ Regressions detected on macos-14 for commit 1993c6e

Regression Summary
  • Audit: 3
  • EIO: 3
  • Table Big Diffs: 3
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • ZSZ Big Diffs: 1
  • Table String Diffs: 1

@joseph-robertson
Copy link
Copy Markdown
Collaborator Author

Does it seem like InternalHeatGains.scc is the appropriate place nowadays to put autosizing code for this object? The code pre-dates much refactor work (e.g., state).

@rraustad

Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
if (state.dataEnvrn->CurEnvirNum != ScannedEnvirNum) {
ScannedEnvirNum = state.dataEnvrn->CurEnvirNum;
DesDayOutDryBulbTemp(ScannedEnvirNum) = state.dataEnvrn->OutDryBulbTemp;
if (state.dataEnvrn->CurEnvirNum <= state.dataEnvrn->TotDesDays) { // FIXME: this was == in the original code
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What original code? I don't see any code that was deleted. I think == was used to determine when all design days have simulated (i.e., components don't usually size until zone sizing is complete). But each design day has multiple time steps so just looking at the CurEnvirNum isn't enough to know that sizing has completed. Most sizing routines are not called until !state.dataGlobal->SysSizingCalc. This does seem weird in that state.dataEnvrn->OutDryBulbTemp is set by the weather manager each time step, so this code is capturing the last time step's OAT in each specific design day. Not what I would expect. Also, at line 6926 below it's looking for max OAT not the min (i.e., if (MinDesOutTemp > DesDayOutDryBulbTemp(Loop))). Too much to discuss in a comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I was just referring to the original code in #4236.

I tried introducing !state.dataGlobal->ZoneSizingCalc in d994859. It brings the InitInternalHeatGains call down from "Initializing Internal Heat Gains" to "Initializing Simulation", which I think aligns with the other baseboard type.

From what I can tell the code is permitting input of a design zone heating setpoint (high temperature), but then looking to design day (days?) for a low temperature? But as you point out, looking at state.dataEnvrn->OutDryBulbTemp which would be the design day's last timestep's OAT; is there a precendence for this kind of weather info gathering, or why couldn't low temperature just be defaulted/input like high temperature?

I'll dig around and see how infiltration and ventilation MCPI and MCPV are used in other places.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What would typically happen in a sizing routine is the component would look to FinalZoneSizing data for information needed for sizing (which means you don't need to gather data as done here with DesDayOutDryBulbTemp(ScannedEnvirNum)). In this case it would be FinalZoneSizing(zn).OutTempAtHeatPeak but using that data would need to be tested to make sure the size of the baseboard was sufficient to meet the peak baseboard load. For example, what is the total infiltration and ventilation load at the peak heating timestep? Is there a time where infiltration and ventilation would be larger and therefore the baseboard would not have sufficient capacity to meet the load? There is also an array of this same data using the min of CalcFinalZoneSizing(zn).HeatOutTempSeq(ts) which would give the minimum OAT for the peak heating design day. There are always questions of this nature regarding which sizing data is appropriate for component sizing (e.g., the peak heating load may not correspond to the peak infiltration or ventilation load).

Copy link
Copy Markdown
Collaborator Author

@joseph-robertson joseph-robertson May 21, 2026

Choose a reason for hiding this comment

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

@rraustad I made several updates based on your suggestions. Using state.dataSize->FinalZoneSizing(NZ).OutTempAtHeatPeak seems to give me the appropriate design size low temperature.

On the topic of infiltration/ventilation MCPI and MCPV, because they are timestep variables and we'd want the peak, could I just add new peak variables in ZoneEquipmentManager analogous to OutTempAtHeatPeak? But instead of using HeatOutTempSeq(ts), it would use MCPI/MCPV. (I don't totally understand how the original implementation of this code dealt/found peaks of MCPV/MCPV.)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What you have done so far looks good by tracking HeatMCPI(V) and HeatMCPI(V)Seq.

@rraustad
Copy link
Copy Markdown
Collaborator

rraustad commented May 11, 2026

Does it seem like InternalHeatGains.scc is the appropriate place nowadays to put autosizing code for this object? The code pre-dates much refactor work (e.g., state).

@rraustad

The BaseBoards do have a size function, but this object is managed in InternalHeatGains, for whatever reason. Sizing is typically called from the init function with proper protection to ensure it is called after sizing completes. So this does seem OK since it's called from InitInternalHeatGains. For zone equipment the protection could use !state.dataGlobal->ZoneSizingCalc (where FinalZoneSizing is complete and available) but most components wait for all sizing to complete.

void InitBaseboard()

    if (!state.dataGlobal->SysSizingCalc && baseboard->baseboards(BaseboardNum).MySizeFlag ) {
        // for each coil, do the sizing once.
        SizeElectricBaseboard(state, BaseboardNum);
        baseboard->baseboards(BaseboardNum).MySizeFlag = false;
    }

So this new code does call a sizing function, it's called from init in the code that manages the object, and the sizing call should be protected to only call the new size function after sizing is complete. Using !OutTempScanNotDone as protection could work as intended, since this appears to only be interested in design day OAT and surface properties, but it doesn't look to me as if that's the case. Also, infiltration and ventilation MCPI and MCPV are also time step variables so something else will need to be used to get the peak of those data.

Comment on lines +7114 to +7118
Real64 NominalUwithConvCoeffs = 0.0;
if (surf.Construction > 0 && surf.Construction <= state.dataHeatBal->TotConstructs) {
bool isWithConvCoefValid = false;
NominalUwithConvCoeffs = DataHeatBalance::ComputeNominalUwithConvCoeffs(state, SurfNum, isWithConvCoefValid);
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've replaced UNomFilm in the old code with NominalUwithConvCoeffs.

Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
Comment on lines +7127 to +7128
ZnInfilSensLoad = state.dataSize->FinalZoneSizing(NZ).MCPIAtHeatPeak * DeltaTMax;
ZnVentSensLoad = state.dataSize->FinalZoneSizing(NZ).MCPVAtHeatPeak * DeltaTMax;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm using the new peak variables for MCPI and MCPV here.

Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
LowTemperatureDes = state.dataSize->FinalZoneSizing(NZ).OutTempAtHeatPeak;
if (IsAutosize) {
thisBBHeat.LowTemperature = LowTemperatureDes;
BaseSizer::reportSizerOutput(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm a little unclear whether I should be using BaseSizer::reportSizerOutput directly or some other class/method.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Either call a dedicated function in the sizer instead of sizing here (see All_Simple_Sizing, and associated calling sites) or call reportSizerOutput. Either method is fine.

autosize, !- Capacity at Low Temperature {W}
autosize, !- Low Temperature {C}
autosize, !- Capacity at High Temperature {W}
autosize, !- High Temperature {C}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

From the docs for Field: High Temperature, "If autosized, this is equal to the design zone heating setpoint temperature described below, so that the capacity at high temperature is zero."

So seeing a zero "Design Size Capacity at High Temperature [W]" value for "SPACE5-1 BBHEAT 1" seems consistent/expected.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This just seems a little odd. You don't but a baseboard (or any other component) in a building and then say "what is the capacity at a high entering air temperature" do you? Because it's obvious. Leave this for now as to not add scope creep.

Comment on lines +6888 to +6892
if (!state.dataGlobal->ZoneSizingCalc && state.dataHeatBal->ZoneBBHeat(Loop).MySizeFlag) {
SizeOaControlledBaseboard(state, Loop);
state.dataHeatBal->ZoneBBHeat(Loop).MySizeFlag = false;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Much much cleaner and simpler than the original code. Thanks for pointing me in the right direction, @rraustad.

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 0c0fdce

Regression Summary
  • Audit: 3
  • EIO: 3
  • ERR: 1
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3
  • Table String Diffs: 1

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 0c0fdce

Regression Summary
  • Audit: 3
  • EIO: 3
  • ERR: 1
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3
  • Table String Diffs: 1


if (thisBBHeat.LowTemperature == DataSizing::AutoSize) {
IsAutosize = true;
}
Copy link
Copy Markdown
Collaborator

@rraustad rraustad May 22, 2026

Choose a reason for hiding this comment

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

Oops, I forgot to tell you an important point about sizing functions. The user could hard size the inputs and NOT perform a sizing run. In this case FinalZoneSizing will not be allocated and line 7034 will crash. Inside the above check add:

CheckZoneSizing(state, CompType, thisBBHeat.Name);

The simulation will fatal out if a sizing run is not performed.

Then add right after line 7032:

bool SizingDesRunThisZone = false;
CheckThisZoneForSizing(state, state.dataSize->CurZoneEqNum, SizingDesRunThisZone);

And you can use this flag to protect for FinalZoneSizing being allocated. Check other zone equipment sizing routines to get a feel for this. Also run a test file with and without zone sizing to check for crashes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Or you could add another struct variable as bool anyInputAutosized that is set in getInput and just call CheckZoneSizing once at the top of the sizing function.

Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
"Design Size Low Temperature [C]",
LowTemperatureDes,
"User-Specified Low Temperature [C]",
LowTemperatureUser);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This call is specific for when the user hard sizes an input AND a sizing run is performed. If a sizing run is not performed you won't have LowTemperatureDes available to report. Also you might not want to report to the eio when hardsized because the user already knows the size of the baseboard. You can report user specified inputs if you like and I don't have any issue with that, in fact I think all autosizable inputs should be reported to the eio regardless of whether or not they are autosized. You just have to add logic to report the right information when appropriate. i.e., you use this call when hard sized AND a sizing run is performed. You use line 7037 when hard sized and a sizing run is not performed and also change Design Size to User-Specified.

Copy link
Copy Markdown
Collaborator Author

@joseph-robertson joseph-robertson May 26, 2026

Choose a reason for hiding this comment

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

Thanks. I've captured this and your other comment in e5aeb46.

But how does state.dataSize->CurZoneEqNum get incremented? It looks like it remains at a value of zero at the point of calling the baseboard sizing routine, which then doesn't turn SizingDesRunThisZone to true.

Edit: Oh. It's OA controlled, so maybe this is intentional. But any idea what the path forward should be then? Maybe this?

Copy link
Copy Markdown
Collaborator

@rraustad rraustad May 28, 2026

Choose a reason for hiding this comment

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

@joseph-robertson you can probably use this method to set the current zone variable used in sizing methods:

    state.dataSize->CurZoneEqNum = Util::FindItemInList(ZoneName, state.dataHeatBal->Zone);

You may have to save CurZoneEqNum and reset after your new sizing function so as not to break anything but I'm not sure that would be necessary, it depends on calling order and whether some other zone component would also be calling sizing. If CurZoneEqNum was not actually set (it's set/reset only in 1 place in ZoneEquipmentManager) before calling your sizing function you probably don't have to worry about resetting this variable.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 3774d15

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 3774d15

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 485b5cb

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 485b5cb

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit d51a14f

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit d51a14f

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 780c900

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 780c900

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 32fed3f

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 32fed3f

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 147e354

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 147e354

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

Joe Robertson added 2 commits June 1, 2026 13:03
Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
Comment on lines +7065 to +7073
BaseSizer::reportSizerOutput(state,
CompType,
CompName,
"Design Size " + SizingString,
LowTemperatureDes,
"User-Specified " + SizingString,
thisBBHeat.LowTemperature);
if (state.dataGlobal->DisplayExtraWarnings) {
if (std::abs(LowTemperatureDes - thisBBHeat.LowTemperature) > state.dataSize->AutoVsHardSizingDeltaTempThreshold) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Stick with calling reportSizerOutput for temperature fields (vs. using HeatingCapacitySizer) -- the AutoVsHardSizingDeltaTempThreshold variable implies a delta check, and also I'm not sure HeatingCapacitySizer would even support negative temperature values.

Comment thread src/EnergyPlus/InternalHeatGains.cc Outdated
}

ZoneEqSizing.HeatingCapacity = true;
ZoneEqSizing.DesHeatingLoad = CapatLowTemperatureDes;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I believe we are essentially overriding DesHeatingLoad with our own sizing value here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

⚠️ Regressions detected on macos-14 for commit 136de41

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 136de41

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread tst/EnergyPlus/unit/InternalHeatGains.unit.cc
Comment thread src/EnergyPlus/InternalHeatGains.cc
@joseph-robertson joseph-robertson added the Documentation Related primarily on the LaTeX-based EnergyPlus documentation label Jun 2, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

⚠️ Regressions detected on macos-14 for commit 91a8ce5

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 91a8ce5

Regression Summary
  • Audit: 3
  • EIO: 3
  • MDD: 1
  • MTD: 1
  • ESO Big Diffs: 1
  • MTR Big Diffs: 1
  • Table Big Diffs: 3

@joseph-robertson joseph-robertson marked this pull request as ready for review June 2, 2026 18:50
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 Documentation Related primarily on the LaTeX-based EnergyPlus documentation IDDChange Code changes impact the IDD file (cannot be merged after IO freeze)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZoneBaseboard:OutdoorTemperatureControlled autosizing is missing

4 participants