Skip to content

Fix heating and cooling runtime fraction for AirLoopHVAC:UnitarySystem for airflow network simulations#11491

Open
lymereJ wants to merge 4 commits intodevelopfrom
fix_unitary_system_afn_runtime_fracs
Open

Fix heating and cooling runtime fraction for AirLoopHVAC:UnitarySystem for airflow network simulations#11491
lymereJ wants to merge 4 commits intodevelopfrom
fix_unitary_system_afn_runtime_fracs

Conversation

@lymereJ
Copy link
Copy Markdown
Collaborator

@lymereJ lymereJ commented Mar 31, 2026

Pull request overview

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

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

@lymereJ lymereJ added the Defect Includes code to repair a defect in EnergyPlus label Mar 31, 2026
@lymereJ
Copy link
Copy Markdown
Collaborator Author

lymereJ commented Mar 31, 2026

Here's a comparison of the duct conduction heat gains for the defect file with the "reference" file with this branch:
image

Also, as expected their energy use is virtually the same:

  • OPT4 (AirLoopHVAC:UnitarySystem):
image
  • OPT3 (AirLoopHVAC:UnitaryHeatCool):
image

Copy link
Copy Markdown
Collaborator Author

@lymereJ lymereJ left a comment

Choose a reason for hiding this comment

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

Code walk-through:

// Report the current output
this->reportUnitarySystem(state, AirLoopNum);

// Get the actual maximum RTF for AFN simulations
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.

Similar changes as for #11367. We save the RTFs at the system level instead of at the coil level and make the determination here, that way we can also include the supplemental heating coil.

int CompIndex = this->m_HeatingCoilIndex;
HVAC::FanOp fanOp = this->m_FanOpMode;
Real64 DesOutTemp = this->m_DesiredOutletTemp;

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.

These were only applied when the system is controller to a setpoint. The propose changes apply to both load-based and setpoint control.

latOut);

// Check that the runtime fraction is less than one so the impact of cycling fan is correctly accounted for in the AFN
EXPECT_TRUE(state->dataAirLoop->AirLoopAFNInfo(1).AFNLoopHeatingCoilMaxRTF < 1);
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.

New unit test to make sure that the correct RTF is retrieved. This verification fails on develop.

@github-actions
Copy link
Copy Markdown

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

Regression Summary
  • ESO Small Diffs: 703
  • Table Small Diffs: 391
  • MTR Small Diffs: 523
  • Table String Diffs: 186
  • EIO: 396
  • JSON Small Diffs: 2
  • ZSZ Small Diffs: 72
  • Table Big Diffs: 36
  • ERR: 14
  • MTR Big Diffs: 2
  • EDD: 4
  • ESO Big Diffs: 10
  • SSZ Small Diffs: 13
  • JSON Big Diffs: 2

@lymereJ lymereJ added the AirflowNetwork Related primarily on airflow-network portions of the codebase label Apr 1, 2026

// Get the actual maximum RTF for AFN simulations
if (state.afn->distribution_simulated && this->m_sysType != SysType::PackagedAC && this->m_sysType != SysType::PackagedHP &&
this->m_sysType != SysType::PackagedWSHP && AirLoopNum > 0) {
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.

I think there may be a problem with these conditionals. The first is that we should move away from checking for the "Packaged" sysType because it was added to reduce diffs. These special considerations should be removed slowly and will cause little diffs so should probably be done 1 at a time. What is important here is that this is a system level object. Not zone equipment, OA sys equipment, or in a ZoneHVAC:OutdoorAirUnit object. The second is that AirLoopNum > 0 will also be true (I think) for a UnitarySystem in the OutdoorAir system.

This conditional would be the same as:

if (state.afn->distribution_simulated && !this->IsZoneEquipment && AirLoopNum > 0) {

except the snag that this system could be called from MixedAir and be part of an outdoor air system. You could probably use this to capture a system object (air loop equipment):

if (state.afn->distribution_simulated && AirLoopNum > 0 && state.dataSize->CurOASysNum == 0) {

I am not positive that AFN would not include OA systems here but if so then CurOASysNum==0 would not be needed (I think it's needed).

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 see, thanks. I was following the existing pattern. Should these mods be done as a follow up PR, or should I include them here?

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.

Just include them here. This is a small change that also gets rid of some "Packaged" sysType checks.

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 made the changes.

if (state.afn->distribution_simulated && this->m_sysType != SysType::PackagedAC && this->m_sysType != SysType::PackagedHP &&
this->m_sysType != SysType::PackagedWSHP && AirLoopNum > 0) {
refAFNLoopHeatingCoilMaxRTF = state.dataAirLoop->AirLoopAFNInfo(AirLoopNum).AFNLoopHeatingCoilMaxRTF;
refAFNLoopCoolingCoilMaxRTF = state.dataAirLoop->AirLoopAFNInfo(AirLoopNum).AFNLoopDXCoilRTF;
Copy link
Copy Markdown
Collaborator

@rraustad rraustad Apr 1, 2026

Choose a reason for hiding this comment

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

Can't you just move these last 2 lines down inside of the block below?

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 don't think so because I think this needs to happen before controlUnitarySystemtoSP or controlUnitarySystemtoLoad are called.

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.

I see. Because these same AFN variables are set in the child. In this parent there can be 2 heating coils so the larger RTF should be used and calling the control functions will set AFNLoopHeatingCoilMaxRTF to the RTF of the last coil called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AirflowNetwork Related primarily on airflow-network portions of the codebase Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect heating energy use for AirLoopHVAC:UnitarySystem in AFN simulation with distribution losses

4 participants