Skip to content

ZoneBaseboard:OutdoorTemperatureControlled autosizing is missing#11579

Draft
joseph-robertson wants to merge 5 commits into
developfrom
zone-baseboard-autosizing
Draft

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

Conversation

@joseph-robertson
Copy link
Copy Markdown
Collaborator

Pull request overview

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

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.

@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.

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 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

3 participants