Skip to content

Fix HybridModel:Zone to Consume People Activity Schedule - #11854

Merged
mitchute merged 4 commits into
developfrom
fix-people-activity-schedule
Oct 10, 2026
Merged

Fix HybridModel:Zone to Consume People Activity Schedule#11854
mitchute merged 4 commits into
developfrom
fix-people-activity-schedule

Conversation

@mitchute

@mitchute mitchute commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Pull request overview

HybridModel:Zone loads the optional people activity schedule in InverseModelHumidity(); however, the people activity schedule value never gets used. This value from the schedule is read here:

EnergyPlus/src/EnergyPlus/ZoneTempPredictorCorrector.cc

Line 5132 in 01c3cba

zone.ZonePeopleActivityLevel = (hmZone.peopleActivityLevelSched != nullptr) ? hmZone.peopleActivityLevelSched->getCurrentVal() : 0.0;

But then the local variable never gets updated. As a result, the default value of 130 W/person is always used regardless of the schedule.

EnergyPlus/src/EnergyPlus/ZoneTempPredictorCorrector.cc

Lines 5142 to 5144 in 01c3cba

if (ActivityLevel <= 0.0) {
ActivityLevel = 130.0;
}

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

mitchute
mitchute commented Oct 7, 2026
View reviewed changes
Comment thread src/EnergyPlus/ZoneTempPredictorCorrector.cc
Comment on lines 5139 to 5145
if (FractionSensible <= 0.0) {
FractionSensible = 0.6;
}

if (ActivityLevel <= 0.0) {
ActivityLevel = 130.0;
}

@mitchute mitchute Oct 7, 2026

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.

There's a few hardcoded default values here and in other places related to this that probably should be elevated to a constexpr and put somewhere.

EnergyPlus/src/EnergyPlus/ZoneTempPredictorCorrector.cc

Line 4964 in 01c3cba

ActivityLevel = 130.0;

EnergyPlus/src/EnergyPlus/ZoneTempPredictorCorrector.cc

Line 5143 in 01c3cba

ActivityLevel = 130.0;

EnergyPlus/src/EnergyPlus/HybridModel.cc

Line 337 in 01c3cba

std::format("Field \"{}\": default people activity level is not provided, default value of 130W/person will be used.",

I think I'll do that next.

mitchute
mitchute commented Oct 7, 2026
View reviewed changes
Comment thread src/EnergyPlus/ZoneTempPredictorCorrector.cc
zone.ZonePeopleRadiantHeatFraction = (hmZone.peopleRadiantFracSched != nullptr) ? hmZone.peopleRadiantFracSched->getCurrentVal() : 0.0;

Real64 FractionSensible = zone.ZonePeopleSensibleHeatFraction;
ActivityLevel = zone.ZonePeopleActivityLevel;

@mitchute mitchute Oct 7, 2026

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.

This one is the fix

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 7c26a2b

Regression Summary
  • ESO Big Diffs: 1

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit 7c26a2b

Regression Summary
  • ESO Big Diffs: 1

@mitchute mitchute added the Defect Includes code to repair a defect in EnergyPlus label Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit 8202304

Regression Summary
  • ESO Big Diffs: 1

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 8202304

Regression Summary
  • ESO Big Diffs: 1

@mitchute mitchute changed the title Fix InverseModelHumidity to Consume People Activity Schedule Fix HybridModel:Zone to Consume People Activity Schedule Oct 10, 2026
@mitchute

mitchute commented Oct 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Example diffs below for HybridModel_4Zone_Solve_PeopleCount_with_HVAC.

newplot(15)

Using the hybrid inverse model, NumPeople is inversely proportional to the activity level:

EnergyPlus/src/EnergyPlus/ZoneTempPredictorCorrector.cc

Line 5176 in a0d220e

Real64 NumPeople = min(UpperBound, max(0.0, LatentGainPeople / (ActivityLevel * (1.0 - FractionSensible))));

Before, the activity level was defaulting to 130. In this branch the file now reading the scheduled value of 120, for a difference of about +8% (130/120 ~ 1.083) is expected.

@github-actions

github-actions Bot commented Oct 10, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 1484afa

Regression Summary
  • ESO Big Diffs: 1

@github-actions

github-actions Bot commented Oct 10, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit 1484afa

Regression Summary
  • ESO Big Diffs: 1

@mitchute

mitchute commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

As expected, minor diffs (below in pct).

Screenshot from 2026-10-09 19-01-26 Screenshot from 2026-10-09 19-01-13

@mitchute

mitchute commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

Ready to go. Merging.

@mitchute
mitchute merged commit 6c3973a into develop Oct 10, 2026
8 checks passed
@mitchute
mitchute deleted the fix-people-activity-schedule branch October 10, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mitchute @nrel-bot-2c