Skip to content

WindowManager Array Transition (part 1) #11432

Merged
mitchute merged 19 commits intodevelopfrom
WindowManager2
Mar 3, 2026
Merged

WindowManager Array Transition (part 1) #11432
mitchute merged 19 commits intodevelopfrom
WindowManager2

Conversation

@amirroth
Copy link
Copy Markdown
Collaborator

@amirroth amirroth commented Feb 25, 2026

Pull request overview

This is a refactor that moves some arrays in WindowManager from Array1D<> and Array2D<> to std::array<> and std::array<std::array<>>, transitions 1-based loops and indexing to 0-based loops and indexing, and moves some variables from state to local scope. Completing the array transition will probably take one more PR.

There are no diffs.

@amirroth amirroth self-assigned this Feb 25, 2026
@amirroth amirroth added Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Feb 25, 2026
@amirroth amirroth added this to the EnergyPlus 26.1 milestone Feb 25, 2026
@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit c7dc965

Regression Summary
  • EIO: 3
  • ESO Small Diffs: 1
  • MTR Small Diffs: 1
  • Table Small Diffs: 1
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit c7dc965

Regression Summary
  • EIO: 3
  • ESO Small Diffs: 1
  • MTR Small Diffs: 1
  • Table Small Diffs: 1
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 1f95993

Regression Summary
  • EIO: 3
  • ESO Small Diffs: 1
  • MTR Small Diffs: 1
  • Table Small Diffs: 1
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 1f95993

Regression Summary
  • EIO: 3
  • ESO Small Diffs: 1
  • MTR Small Diffs: 1
  • Table Small Diffs: 1
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

rbt = wm->rbop[0][n - 1];
tt = top[n - 1][0];
rft = rfop[n - 1][0];
rbt = rbop[0][n - 1];
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.

Looking a the issue you found using bisect just above here (1944), why wasn't the index of top, rfop, and rbop also changed here (1951 - 1953)?

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 found it.

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit dc0cc45

Regression Summary
  • EIO: 2
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 28a4e4a

Regression Summary
  • EIO: 353
  • ERR: 82
  • ESO Big Diffs: 332
  • Table Big Diffs: 341
  • Table String Diffs: 309
  • MTR Big Diffs: 258
  • SSZ Big Diffs: 225
  • ZSZ Big Diffs: 247
  • MAP: 4
  • JSON Big Diffs: 1
  • Audit: 2
  • ESO Small Diffs: 9
  • DELightIn: 1
  • DELightOut: 1
  • EDD: 6
  • MTR Small Diffs: 21
  • PERF_LOG: 1

@github-actions
Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 467bc0a

Regression Summary
  • EIO: 2
  • ESO Big Diffs: 2
  • Table Big Diffs: 2

Copy link
Copy Markdown
Collaborator Author

@amirroth amirroth left a comment

Choose a reason for hiding this comment

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

Quick code walk-through.


auto &wm = state.dataWindowManager;
Array1D<Real64> sabsPhi(nume); // Glazing system absorptance for a glass layer
std::array<Real64, nume> sabsPhi; // Glazing system absorptance for a glass layer
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.

Many changes like this.

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.

Good

wm->lSimpleGlazingSystem = false;


bool lSimpleGlazingSystem = 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.

These can be local variables.

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.

👍

// from properties at normal incidence
for (int IGlass = 1; IGlass <= NGlass; ++IGlass) {
LayPtr = thisConstruct.LayerPoint(wm->LayerNum[IGlass - 1]);
for (int iGlass = 0; iGlass < NGlass; ++iGlass) {
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.

0-based loops

// Loop over glass layers in the construction
for (int IGlass = 1; IGlass <= NGlass; ++IGlass) {
int LayNum = 1 + 2 * (IGlass - 1);
for (int iGlass = 0; iGlass < NGlass; ++iGlass) {
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.

0-based indexing.

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.

👍 ++

rbt = wm->rbop[0][n - 1];
tt = top[n - 1][0];
rft = rfop[n - 1][0];
rbt = rbop[0][n - 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.

I found it.

Comment thread src/EnergyPlus/WindowManager.cc Outdated
for (int IGlass = 1; IGlass <= NGlass; ++IGlass) {
constr.AbsDiff(IGlass) = ShadeTrans * ShadeReflFac * solabsDiff(IGlass);
for (int iGlass = 0; iGlass < NGlass; ++iGlass) {
constr.AbsDiff(iGlass+1) = ShadeTrans * ShadeReflFac * solabsDiff[iGlass];
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.

Will transition the corresponding arrays in Construction next.


RhoAir = wm->AirProps[0] + wm->AirProps[1] * (TGapOld - Constant::Kelvin);
ViscAir = wm->AirProps[4] + wm->AirProps[5] * (TGapOld - Constant::Kelvin);
RhoAir = AirDens + AirDDensDT * (TGapOld - Constant::Kelvin);
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 should not be in an array, they should be discrete named constants.

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.

Good catch


namespace Window {

Real64 constexpr AirDens = 1.29;
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.

Not sure why these were previously in an array.

Real64 A67 = 0.0;

// TEMP MOVED FROM DataHeatBalance.hh -BLB

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.

Made local.

bool HasWindows = false;
bool HasComplexWindows = false;
bool HasEQLWindows = false; // equivalent layer window defined
Real64 SimpleGlazingSHGC = 0.0; // value of SHGC for simple glazing system block model
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.

In general try to reduce the number of state variables unless they are actually state that persists across time steps. Don't use state to pass information from one function to another within the same timestep, that's what local variables and arguments are for.

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.

Noted. Thanks.

Copy link
Copy Markdown
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

I just ran a few spot checks after merging in develop, and this is ready to go. GitHub is experiencing issues with actions currently, so we'll hold of on merging this until that is resolved.


auto &wm = state.dataWindowManager;
Array1D<Real64> sabsPhi(nume); // Glazing system absorptance for a glass layer
std::array<Real64, nume> sabsPhi; // Glazing system absorptance for a glass layer
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.

Good

wm->lSimpleGlazingSystem = false;


bool lSimpleGlazingSystem = false;
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.

👍

// Loop over glass layers in the construction
for (int IGlass = 1; IGlass <= NGlass; ++IGlass) {
int LayNum = 1 + 2 * (IGlass - 1);
for (int iGlass = 0; iGlass < NGlass; ++iGlass) {
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.

👍 ++


RhoAir = wm->AirProps[0] + wm->AirProps[1] * (TGapOld - Constant::Kelvin);
ViscAir = wm->AirProps[4] + wm->AirProps[5] * (TGapOld - Constant::Kelvin);
RhoAir = AirDens + AirDDensDT * (TGapOld - Constant::Kelvin);
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.

Good catch

bool HasWindows = false;
bool HasComplexWindows = false;
bool HasEQLWindows = false; // equivalent layer window defined
Real64 SimpleGlazingSHGC = 0.0; // value of SHGC for simple glazing system block model
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.

Noted. Thanks.

@mitchute
Copy link
Copy Markdown
Collaborator

mitchute commented Mar 3, 2026

OK. Actions appear to be back up and running. Merging.

@mitchute mitchute merged commit 92a6e37 into develop Mar 3, 2026
11 checks passed
@mitchute mitchute deleted the WindowManager2 branch March 3, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NotIDDChange Code does not impact IDD (can be merged after IO freeze) Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants