Skip to content

Correct SizingMethod variable initialization#11461

Closed
rraustad wants to merge 5 commits intodevelopfrom
11460-SizingMethod
Closed

Correct SizingMethod variable initialization#11461
rraustad wants to merge 5 commits intodevelopfrom
11460-SizingMethod

Conversation

@rraustad
Copy link
Collaborator

@rraustad rraustad commented Mar 12, 2026

Pull request overview

Description of the purpose of this PR

The UnitarySystem model uses scalable sizing inputs that need to be correctly initialized according to user inputs.

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

@rraustad rraustad added the Defect Includes code to repair a defect in EnergyPlus label Mar 12, 2026
@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 3552d2c

Regression Summary
  • Audit: 5
  • EIO: 12
  • Table Big Diffs: 12
  • ERR: 4
  • ESO Big Diffs: 6
  • Table String Diffs: 7
  • MTR Big Diffs: 4
  • MTR Small Diffs: 1

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 3552d2c

Regression Summary
  • Audit: 5
  • EIO: 12
  • Table Big Diffs: 12
  • ERR: 4
  • ESO Big Diffs: 6
  • Table String Diffs: 7
  • MTR Big Diffs: 4
  • MTR Small Diffs: 1

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit c3581a8

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit c3581a8

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@rraustad rraustad changed the title Step 1: See if this breaks anything Correct SizingMethod variable initialization Mar 12, 2026
if (this->m_HeatPump) {
EqSizing.SizingMethod(HVAC::HeatingAirflowSizing) = HeatingSAFlowMethod;
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really can't believe that this type of code failure (initializing a necessary sizing array variable) lasted this long. It could be that some clean up or refador caused this but in the end this seems to fix it, along with the 2 logic changes below.

// cooling coil air inlet node temp is greater than cooling coil air outlet node temp
EXPECT_GT(state->dataLoopNodes->Node(thisSys->AirInNode).Temp, state->dataLoopNodes->Node(thisSys->AirOutNode).Temp);
EXPECT_NEAR(thisSys->m_CoolingCycRatio, 0.36056, 0.001);
EXPECT_NEAR(thisSys->m_CoolingCycRatio, 0.37971, 0.001);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I assume that a change like this is caused by the code taking a different path now.

DataZoneEquipment::GetZoneEquipmentData(*state); // read zone equipment configuration and list objects

state->dataSize->ZoneEqSizing.allocate(1);
state->dataSize->ZoneEqSizing(1).SizingMethod.allocate(25);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A unit test that does not use ZoneUnitarySysTest fixture (where these are already allocated) will need to allocate these, the 2nd one because of this code change. This unit test uses EnergyPlusFixture.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 7076cfa

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 7076cfa

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@rraustad
Copy link
Collaborator Author

rraustad commented Mar 13, 2026

I do not understand these CI diffs. I do expect potential diffs but I also expect them to match my local results. If I run PackagedTerminalAirConditionerVSAS in this branch and develop I get these results using weather file LOCATION,Miami Intl Ap,FL,USA,TMY3,722020.

develop:

Component Sizing Information, ZoneHVAC:PackagedTerminalAirConditioner, ZONE2PTAC, Design Size Heating Supply Air Flow Rate [m3/s], 0.21487          
Component Sizing Information, ZoneHVAC:PackagedTerminalAirConditioner, ZONE3PTAC, Design Size Heating Supply Air Flow Rate [m3/s], 0.42782          

this branch:

Component Sizing Information, ZoneHVAC:PackagedTerminalAirConditioner, ZONE2PTAC, Design Size Heating Supply Air Flow Rate [m3/s], 0.21487          
Component Sizing Information, ZoneHVAC:PackagedTerminalAirConditioner, ZONE3PTAC, Design Size Heating Supply Air Flow Rate [m3/s], 0.42782          

The CI shows:

- Component Sizing Information, ZoneHVAC:PackagedTerminalHeatPump, ZONE2PTHP, Design Size Heating Supply Air Flow Rate [m3/s], 0.21956          
+ Component Sizing Information, ZoneHVAC:PackagedTerminalHeatPump, ZONE2PTHP, Design Size Heating Supply Air Flow Rate [m3/s], 0.29283          

- Component Sizing Information, ZoneHVAC:PackagedTerminalHeatPump, ZONE3PTHP, Design Size Heating Supply Air Flow Rate [m3/s], 0.34232          
+ Component Sizing Information, ZoneHVAC:PackagedTerminalHeatPump, ZONE3PTHP, Design Size Heating Supply Air Flow Rate [m3/s], 0.45655          

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 9ac61ed

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 9ac61ed

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@mitchute
Copy link
Collaborator

I ran git bisect. This started failing due to something in #11432, but I can't narrow it down to a specific commit. That all seems unrelated to the issue at hand anyway, so I'm guessing some cleanup in there exposed this issue.

@rraustad
Copy link
Collaborator Author

rraustad commented Mar 13, 2026

I'm still wrestling with the best logic for these scalable sizing inputs. I think I may have over specified the UnitarySystemModel_ConfirmUnitarySystemSizingTest unit test that exercises these scalable inputs thereby hiding the correct answer. If the code takes the wrong path it will just size with the data I used to back calculate the data used for the unit test (e.g., ends up using FinalZoneSizing data instead of the scaled value). I'll try to get this as sound as possible knowing that this area needs attention.

@rraustad
Copy link
Collaborator Author

I don't think 11432 is at all related. Changes to WindowManager should not affect component sizing.

@mitchute
Copy link
Collaborator

I don't think 11432 is at all related. Changes to WindowManager should not affect component sizing.

Agreed

SysCoolingFlow = CoolCapAtPeak * this->m_MaxCoolAirVolFlow;
state.dataSize->DataTotCapCurveIndex = 0;
state.dataSize->DataFlowPerCoolingCapacity = this->m_MaxCoolAirVolFlow;
state.dataSize->DataAutosizedCoolingCapacity = CoolCapAtPeak;
Copy link
Collaborator Author

@rraustad rraustad Mar 13, 2026

Choose a reason for hiding this comment

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

These are the globals used in the switch block for FlowPerCoolingCapacity, so set them and use that block in the switch on the final sizing pass. I still don't like things in the sizer functions, like heating used in a cooling function and using a std::max when sizing a fractional cooling air flow, but this is a good step forward. I do recall that these if conditionals were added just to be safe but it seems like over specifying code. See CoolingAirFlowSizing as an example.

@github-actions
Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 87b71ca

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 87b71ca

Regression Summary
  • Audit: 5
  • EIO: 10
  • Table Big Diffs: 10
  • ERR: 2
  • ESO Big Diffs: 5
  • Table String Diffs: 5
  • MTR Big Diffs: 4

@rraustad
Copy link
Collaborator Author

OutdoorAirUnit has some new warnings and different component sizing so that file needs to be looked at before this can merge.

@rraustad
Copy link
Collaborator Author

The OutdoorAirUnit example file has CoilSystem:Cooling:DX objects in the outdoor air unit equipment list. This object is managed by UnitarySystem so the changes in this branch stomp on OA Unit sizing. I still think there is an issue with how the SizingMethod is handled in UnitarySystem but changes like this seem too sweeping to make this type of change at this time.

ZoneHVAC:OutdoorAirUnit:EquipmentList,
  Zone1OAEQLIST,           !- Name
  HeatExchanger:AirToAir:SensibleAndLatent,  !- Component 1 Object Type
  Zone1OAUHR,              !- Component 1 Name
  CoilSystem:Cooling:DX,   !- Component 2 Object Type
  DX Cooling Coil System 1,!- Component 2 Name
  Coil:Heating:Electric,   !- Component 3 Object Type
  Zone1OAUHeatingCoil;     !- Component 3 Name

The outdoor air unit uses these ZoneEqSizing variables which are used in the default for the switch in CoolingAirFlowSizing. To make this code change would require changing other zone equipment sizing methodology and that seems a little much at this point. I will close this PR and make another change that directly fixes the failing unit test.

    state.dataSize->ZoneEqSizing(state.dataSize->CurZoneEqNum).CoolingAirFlow = true;
    state.dataSize->ZoneEqSizing(state.dataSize->CurZoneEqNum).HeatingAirFlow = true;
    state.dataSize->ZoneEqSizing(state.dataSize->CurZoneEqNum).CoolingAirVolFlow = thisOutAirUnit.OutAirVolFlow;
    state.dataSize->ZoneEqSizing(state.dataSize->CurZoneEqNum).HeatingAirVolFlow = thisOutAirUnit.OutAirVolFlow;
    state.dataSize->ZoneEqSizing(state.dataSize->CurZoneEqNum).OAVolFlow = thisOutAirUnit.OutAirVolFlow;

@rraustad
Copy link
Collaborator Author

@mitchute I'm not sure what to do with this branch since it tries to address an issue with UnitarySystem sizing. The scalable sizing inputs do not seem to be handled properly but this type of code change would take more time to flesh out. I think this PR should close but this issue does need to stay on the radar. PR #11464 directly corrects the unit test for now.

@mitchute
Copy link
Collaborator

Thanks @rraustad. That works. I will merge the fix on #11464, close this, and make a separate issue about the scalable sizing inputs pointing back to here for reference.

@mitchute
Copy link
Collaborator

Closing, but I will not delete the branch.

@mitchute mitchute closed this Mar 13, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unintialized SizingMethod variable on Linux 24.04 for "UnitarySystemModel_MultiSpeedDXCoolCoil_Only_NoFan"

3 participants