Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Space Component Loads Report and Fix Zone Component Loads when Enclosures Differ from Zones #10730

Open
wants to merge 37 commits into
base: develop
Choose a base branch
from

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Sep 12, 2024

Pull request overview

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • 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

Reviewer

This will not be exhaustively relevant to every PR.

  • 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

@mjwitte mjwitte added NewFeature Includes code to add a new feature to EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Sep 12, 2024
@mjwitte mjwitte added this to the EnergyPlus 24.2 milestone Sep 12, 2024
Copy link

⚠️ Regressions detected on macos-14 for commit aaf7933

Regression Summary
  • ESO Big Diffs: 1

Copy link

⚠️ Regressions detected on macos-14 for commit 5631bbf

Regression Summary
  • ESO Big Diffs: 1

Copy link

⚠️ Regressions detected on macos-14 for commit ce39eda

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

auto &spCLDayTS = state.dataOutRptTab->spCompLoads[iSpace - 1].day[state.dataSize->CurOverallSimDay - 1].ts[TimeStepInDay - 1];
gatherCompLoadIntGain2(state, spCLDayTS, state.dataHeatBal->space(iSpace).zoneNum, iSpace);
}
}
Copy link
Contributor

@rraustad rraustad Sep 21, 2024

Choose a reason for hiding this comment

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

I recall something about if space heat balance is active, then the zone HB results are overwritten with the sum of the spaces. If that's true then why bother processing the zones at all (line 9511) if those results are overwritten by 9516? Is line 9516 iSpace = 1 to NumOfZones, or 1 to numSpaces?

If (state.dataHeatBal->doSpaceHeatBalanceSizing) {
    gather spaces, 9516 to 9519
} else {
    gather zones, 9511 to 9514
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's possible. Baby steps at this point. Just trying to find all of the places that apply so that the space results are really just for the space.

Copy link

⚠️ Regressions detected on macos-14 for commit 912635e

Regression Summary
  • Table Big Diffs: 2

Copy link

github-actions bot commented Oct 3, 2024

⚠️ Regressions detected on macos-14 for commit 2618270

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

Copy link

github-actions bot commented Oct 4, 2024

⚠️ Regressions detected on macos-14 for commit b91e671

Regression Summary
  • Table Big Diffs: 5

Copy link

⚠️ Regressions detected on macos-14 for commit 7366aec

Regression Summary
  • Table Big Diffs: 5

@mjwitte mjwitte marked this pull request as ready for review October 11, 2024 17:21
@mjwitte mjwitte added Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring Defect Includes code to repair a defect in EnergyPlus labels Oct 11, 2024
@mjwitte mjwitte changed the title Space Component Loads Report Add Space Component Loads Report and Fix Zone Component Loads when Enclosures Differ from Zones Oct 11, 2024
Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

Refactoring code walkthru . . .

@@ -53,7 +53,6 @@

// ObjexxFCL Headers
#include <ObjexxFCL/Array.functions.hh>
#include <ObjexxFCL/Array3D.hh>
Copy link
Contributor Author

@mjwitte mjwitte Oct 11, 2024

Choose a reason for hiding this comment

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

Removed several stray includes that were no longer being used.

Comment on lines -14943 to -14944
Array1D<Real64> peopleDelaySeqHeat;
Array1D<Real64> peopleDelaySeqCool;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These arrays are temporary and used one at a time, collapsed into a single set to use for both heat and cool.

Comment on lines +494 to +513
struct compLoadsSurface
{
Real64 loadConvectedNormal = 0.0;
Real64 loadConvectedWithPulse = 0.0;
Real64 netSurfRadSeq = 0.0;
Real64 ITABSFseq = 0.0; // used for determining the radiant fraction on each surface
Real64 TMULTseq = 0.0; // used for determining the radiant fraction on each surface
Real64 lightSWRadSeq = 0.0; // short wave visible radiation
Real64 feneSolarRadSeq = 0.0;
};

struct compLoadsTimeStepSurfaces
{
std::vector<compLoadsSurface> surf;
};

struct componentLoadsSurf
{
std::vector<compLoadsTimeStepSurfaces> ts;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New sets of nested vectors to replace a bunch of loose Array3D. This is for surfaces, similar ones for zone/spaces, and enclosures.

Comment on lines -1158 to -1160
Array3D<Real64> loadConvectedNormal;
Array3D<Real64> loadConvectedWithPulse;
Array3D<Real64> netSurfRadSeq;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced these and many other Array3D with nested std::vector.

Comment on lines +1267 to +1270
std::vector<OutputReportTabular::componentLoadsSurf> surfCompLoads; // Surface component loads by day, timestep, then surface
std::vector<OutputReportTabular::componentLoadsSpZn> znCompLoads; // Zone component loads by day, timestep, then zone
std::vector<OutputReportTabular::componentLoadsSpZn> spCompLoads; // Space component loads by day, timestep, then space
std::vector<OutputReportTabular::componentLoadsEncl> enclCompLoads; // Enclosure component loads by day, timestep, then enclsoure
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New component loads vectors for surface, zone, space, and enclosure.

Comment on lines -1309 to -1320
int ZoneNumCLCDC = 0;
int SurfNumCLCDC = 0;
int TimeStepCLCDC = 0;
int TimeOfPulseCLCDC = 0;
int CoolDesSelectedCLCDC = 0; // design day selected for cooling
int HeatDesSelectedCLCDC = 0; // design day selected for heating
int iSurfGCLS = 0;
int ZoneNumGCLS = 0;
int TimeStepInDayGCLS = 0;
int iZoneGCLH = 0;
int TimeStepInDayGCLH = 0;
Array3D_bool adjFenDone;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved out of state.

Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

Walkthru for fix #10784 . . .

@@ -9229,6 +9303,32 @@ namespace InternalHeatGains {
return SumRadiationGainRate;
}

Real64 SumEnclosureInternalRadiationGainsByTypes(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New function to sum radiant gains by enclosure.

Comment on lines +9603 to +9610
for (int iEncl = 1; iEncl <= state.dataViewFactor->NumOfRadiantEnclosures; ++iEncl) {
auto &enclCLDayTS = state.dataOutRptTab->enclCompLoads[state.dataSize->CurOverallSimDay - 1].ts[TimeStepInDay - 1].encl[iEncl - 1];
enclCLDayTS.peopleRadSeq = SumEnclosureInternalRadiationGainsByTypes(state, iEncl, IntGainTypesPeople);
enclCLDayTS.lightLWRadSeq = SumEnclosureInternalRadiationGainsByTypes(state, iEncl, IntGainTypesLight);
enclCLDayTS.equipRadSeq = SumEnclosureInternalRadiationGainsByTypes(state, iEncl, IntGainTypesEquip);
enclCLDayTS.hvacLossRadSeq = SumEnclosureInternalRadiationGainsByTypes(state, iEncl, IntGainTypesHvacLoss);
enclCLDayTS.powerGenRadSeq = SumEnclosureInternalRadiationGainsByTypes(state, iEncl, IntGainTypesPowerGen);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Save enclosure gain sequences by type for use later in component load calcs.

lightLWConvFromSurf += ort->lightLWRadSeq(desDaySelected, sourceStep, zoneIndex) * thisQRadThermInAbsMult;
auto &compLoadTS = szCLDay.ts[sourceStep - 1].spacezone[szNumMinus1];
auto const &surfCLDayTS = surfCLDay.ts[sourceStep - 1].surf[jSurf - 1];
auto const &enclCLDayTS = enclCLDay.ts[sourceStep - 1].encl[radEnclosureNum - 1];
Real64 thisQRadThermInAbsMult =
surfCLDayTS.TMULTseq * surfCLDayTS.ITABSFseq * state.dataSurface->Surface(jSurf).Area * decayCurve(mStepBack, jSurf);
peopleConvFromSurf += enclCLDayTS.peopleRadSeq * thisQRadThermInAbsMult;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use enclosure gain sequences now. Previously, zone gains were used which could be high or low depending on how the zones/spaces/enclosures align.

Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

Space component loads walkthru . . .

Comment on lines +2278 to +2289
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) { // Start of zone loads report variable update loop ...
auto &zone = state.dataHeatBal->Zone(zoneNum);
auto &znAirRpt = state.dataHeatBal->ZnAirRpt(zoneNum);
auto &znEquipConfig = state.dataZoneEquip->ZoneEquipConfig(zoneNum);
reportAirHeatBal1(state, znAirRpt, znEquipConfig, zoneNum);
}
if (state.dataHeatBal->doSpaceHeatBalance) {
for (int spaceNum = 1; spaceNum <= state.dataGlobal->numSpaces; ++spaceNum) {
auto &spAirRpt = state.dataHeatBal->spaceAirRpt(spaceNum);
auto &spEquipConfig = state.dataZoneEquip->spaceEquipConfig(spaceNum);
int zoneNum = state.dataHeatBal->space(spaceNum).zoneNum;
reportAirHeatBal1(state, spAirRpt, spEquipConfig, zoneNum, spaceNum);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Break up ReportAirHeatBalance into smaller functions that can be re-used for zones and spaces.

Comment on lines -143 to -144
Array1D<Real64> MixSenLoad; // Mixing sensible loss or gain
Array1D<Real64> MixLatLoad; // Mixing latent loss or gain
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to DataHeatBalance::AirReportVars.

@@ -286,7 +286,7 @@ void GetSimpleAirModelInputs(EnergyPlusData &state, bool &ErrorsFound) // IF err

// Following used for reporting
state.dataHeatBal->ZnAirRpt.allocate(state.dataGlobal->NumOfZones);
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) {
if (state.dataHeatBal->doSpaceHeatBalance) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, this is wrong, should be doSpaceHeatBalanceSimulation || doSpaceHeatBalanceSizing. It might be ok, but not safe if sizing is false and simulation is true. Will fix.

Comment on lines +14744 to +14747
for (int iZoneGCLH = 1; iZoneGCLH <= state.dataGlobal->NumOfZones; ++iZoneGCLH) {
auto &znCompLoadDayTSZone = znCompLoadDayTS.spacezone[iZoneGCLH - 1];
auto &zoneAirRpt = state.dataHeatBal->ZnAirRpt(iZoneGCLH);
gatherSpaceZoneCompLoadsHVAC(znCompLoadDayTSZone, zoneAirRpt, state.dataHVACGlobal->TimeStepSysSec);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another breakup into smaller functions.

int zoneNum = state.dataHeatBal->space(iSpace).zoneNum;
if (!state.dataZoneEquip->ZoneEquipConfig(zoneNum).IsControlled) continue;
if (allocated(state.dataSize->CalcFinalSpaceSizing)) {
computeSpaceZoneCompLoads(state,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern repeats several times, called for either space of zone.

Copy link

⚠️ Regressions detected on macos-14 for commit 85bf54b

Regression Summary
  • Table Big Diffs: 5

Copy link

⚠️ Regressions detected on macos-14 for commit 7957607

Regression Summary
  • Table Big Diffs: 5

Copy link

⚠️ Regressions detected on macos-14 for commit d8e14ca

Regression Summary
  • Table Big Diffs: 5

@Myoldmopar Myoldmopar self-assigned this Dec 4, 2024
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 NewFeature Includes code to add a new feature to EnergyPlus 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.

Zone Component Loads Report may be incorrect when Zones and Enclosures are not the same
7 participants