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

Fix ITE Standard Density Air Volume Flow Rate and Outdoor Air Details OA by Airloop calculation error, should divide by air density #10584

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

yujiex
Copy link
Collaborator

@yujiex yujiex commented Jun 25, 2024

Pull request overview

In the code, the "ITE Standard Density Air Volume Flow Rate" is calculated as follows.

state.dataHeatBal->ZoneITEq(Loop).AirVolFlowStdDensity = AirMassFlowRate * state.dataEnvrn->StdRhoAir;

However, the volume flow rate should be the air mass flow rate divided by the air density, rather than multiplied by. This PR fixes this calculation error.

In the example file

before fixing, the calculated standard air pressure is around 0.8 kg/m3
image

after fixing, the calculated standard air pressure is around 1.2 kg/m3 (the correct value).
image

Regression diffs

ESO diff

1ZoneDataCenterCRAC_wApproachTemp
1ZoneDataCenterCRAC_wApproachTemp_ClassH1
1ZoneDataCenterCRAC_wPumpedDXCoolingCoil
2ZoneDataCenterHVAC_wEconomizer
2ZoneDataCenterHVAC_wEconomizer_ClassA1A2

The diffs are like this (the following is from 1ZoneDataCenterCRAC_wApproachTemp_ClassH1). These diffs are expected as we've changed the calculation of the ITE Standard Density Air Volume Flow Rate.

"Max absolute diff: 0.6383225713838598, field: DATA CENTER SERVERS:ITE Standard Density Air Volume Flow Rate [m3/s](Hourly), time:  12/21  01:00:00, relative: 0.07367292303016482"
"Max relative diff: 0.07367292303016501, field: DATA CENTER SERVERS:ITE Standard Density Air Volume Flow Rate [m3/s](Hourly), time:  07/21  24:00:00, absolute: 0.46079143420247703"

MTD, AUD diffs

1ZoneDataCenterCRAC_wApproachTemp
These are due to the following two added output:variables in the example file:
Output:Variable,,Zone ITE Standard Density Air Volume Flow Rate,Hourly;
Output:Variable,
,Zone ITE Air Mass Flow Rate,Hourly;

Table big diffs

These appear in 329 files. See this csv file attached.
reg_diff_summary.csv
reg_diff_summary_label_table_diff_var.csv

  • The diffs are mostly on "Mechanical Ventilation" and "Total Ventilaiton". These are expected.
  • Some are on "Limits and Scheduled Limits", like in 5ZoneAirCooled.idf
image The value for this table entry is calculated here in `avgFlowRate(iSys, MixedAir::OALimitFactor::Limits)`
 PreDefTableEntry(
                state, state.dataOutRptPredefined->pdchOaAvFctLimit, thisPrimaryAirSys.Name, avgFlowRate(iSys, MixedAir::OALimitFactor::Limits), 4);

The avgFlowRate definition is as follow.

auto avgFlowRate = [&state](int sysNum, MixedAir::OALimitFactor limitingFactorType) {
                int time = state.dataSysRpts->SysPreDefRep(sysNum).TimeAtOALimitOcc[static_cast<int>(limitingFactorType)];
                if (time > 0) {
                    return state.dataSysRpts->SysPreDefRep(sysNum).MechVentTotAtLimitOcc[static_cast<int>(limitingFactorType)] /
                           (time * Constant::SecInHour);
                } else {
                    return 0.0;
                }
            };

The MechVentToAtLimitOcc calculation involves mechVentFlow, which is impacted by this bug fix.

        // set time at OA limiting factors
        if (mechVentFlow > HVAC::SmallAirVolFlow) {
            int thisOAControlNum = state.dataAirLoop->AirLoopControlInfo(sysNum).OACtrlNum;
            if (thisOAControlNum > 0) {
                int limitFactorIndex = static_cast<int>(state.dataMixedAir->OAController(thisOAControlNum).OALimitingFactor);
                thisSysPreDefRep.TimeAtOALimit[limitFactorIndex] += TimeStepSys;
                if (thisSysVentRepVars.AnyZoneOccupied) {
                    thisSysPreDefRep.TimeAtOALimitOcc[limitFactorIndex] += TimeStepSys;
                    thisSysPreDefRep.MechVentTotAtLimitOcc[limitFactorIndex] += mechVentFlow * TimeStepSysSec;
                }
            }
        }

  • Some have diffs in Time Below Voz-sum-dyn, Time At Voz-sum-dyn, or Time Above Voz-sum-dyn. One example is the

image

The following shows the relevant code computing the table entry "Time At Voz-sum-dyn [hr]". The totMechNatVentVolFlowStdRho variable is computed using mechVentFlow, thus this regression diff is expected as well.

 s->pdchOaTaAlTmAt = newPreDefColumn(state, s->pdstOAtotAirByLoop, "Time At Voz-sum-dyn [hr]");
...
 PreDefTableEntry(state, state.dataOutRptPredefined->pdchOaTaAlTmAt, thisPrimaryAirSys.Name, thisSysPreDefRep.TimeAtVozDynTotal);
...
 Real64 totMechNatVentVolFlowStdRho = mechVentFlow + thisSysVentRepVars.NatVentFlow;
...
 } else if (totMechNatVentVolFlowStdRho > HVAC::SmallAirVolFlow) {
            thisSysVentRepVars.TimeAtVozDyn = TimeStepSys;
            thisSysPreDefRep.TimeAtVozDynTotal += TimeStepSys;
}
  • Some have diffs in "Exhaust Flow". One example is the "5ZoneAirCooled_ZoneAirMassFlowBalance.idf"

image

The following are relevant code computing this table entry. As is explained above, the avgFlowRate calculation involves mechVentFlow, which is impacted by this bug fix. This regression diff is expected.

        s->pdchOaAvFctExhaust = newPreDefColumn(state, s->pdstOAavgFactorsDurOcc, "Exhaust Flow [m3/s]");              // todo
        PreDefTableEntry(state,
                          state.dataOutRptPredefined->pdchOaAvFctExhaust,
                          thisPrimaryAirSys.Name,
                          avgFlowRate(iSys, MixedAir::OALimitFactor::Exhaust),
                          4);
  • Some diffs are in "Mixed Air Flow". One example is the "5ZoneCoolingPanelBaseboard.idf"

image

The following are relevant code computing this table entry. As is explained above, the avgFlowRate calculation involves mechVentFlow, which is impacted by this bug fix. This regression diff is expected.

        s->pdchOaAvFctMixedLimit = newPreDefColumn(state, s->pdstOAavgFactorsDurOcc, "Mixed Air Flow [m3/s]");         // todo
        PreDefTableEntry(state,
                          state.dataOutRptPredefined->pdchOaAvFctMixedLimit,
                          thisPrimaryAirSys.Name,
                          avgFlowRate(iSys, MixedAir::OALimitFactor::MixedAir),
                          4);

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

@yujiex yujiex self-assigned this Jun 25, 2024
@mjwitte
Copy link
Contributor

mjwitte commented Jun 25, 2024

@yujiex Have you searched the code for * state.dataEnvrn->StdRhoAir to see if there are any other similar mistakes?

@mjwitte
Copy link
Contributor

mjwitte commented Jun 25, 2024

But that will have lots of hits for converting volume to mass flow.

@yujiex yujiex added the Defect Includes code to repair a defect in EnergyPlus label Jun 25, 2024
@yujiex yujiex changed the title fix ITE Standard Density Air Volume Flow Rate fix ITE Standard Density Air Volume Flow Rate calculation error, should divide by air density Jun 25, 2024
@yujiex
Copy link
Collaborator Author

yujiex commented Jun 25, 2024

@yujiex Have you searched the code for * state.dataEnvrn->StdRhoAir to see if there are any other similar mistakes?

Let me check

@yujiex
Copy link
Collaborator Author

yujiex commented Jun 25, 2024

But that will have lots of hits for converting volume to mass flow.

It seems most places convert volume to mass flow rate, not the other way, so "*" is correct in those places.

@yujiex
Copy link
Collaborator Author

yujiex commented Jun 25, 2024

I looked at ones using "* state.dataEnvrn->StdRhoAir".
The following are all volume flow rate multiplied by density (correct) (note DataAirFlowUsedForSizing, MinOA, SysOAuc are volume flow rate)
image
image
image
image
image
image

The following one might be wrong, as OAFlow is mass flow rate and MechVentFlow is "air loop mechanical vent total volume OA at standard density {m3/s}"

image

@yujiex yujiex requested review from Myoldmopar and mjwitte June 25, 2024 23:41
@mjwitte
Copy link
Contributor

mjwitte commented Jun 26, 2024

The following one might be wrong, as OAFlow is mass flow rate and MechVentFlow is "air loop mechanical vent total volume OA at standard density {m3/s}"

Yes, that one is wrong. It is being fixed in #10518

@mjwitte
Copy link
Contributor

mjwitte commented Jun 26, 2024

But I could close that and let the fix go in here. One less PR to review.

@mjwitte mjwitte changed the title fix ITE Standard Density Air Volume Flow Rate calculation error, should divide by air density Fix ITE Standard Density Air Volume Flow Rate and Outdoor Air Details OA by Airloop calculation error, should divide by air density Jun 26, 2024
@yujiex
Copy link
Collaborator Author

yujiex commented Jun 26, 2024

But I could close that and let the fix go in here. One less PR to review.

that works =)

@Myoldmopar
Copy link
Member

Lots of diffs here, but an obvious fix. I'm inclined to drop this fix right in. @mjwitte any hesitation at this point?

@mjwitte
Copy link
Contributor

mjwitte commented Jul 10, 2024

Lots of diffs here, but an obvious fix. I'm inclined to drop this fix right in. @mjwitte any hesitation at this point?

None here. Full speed ahead, cap'n!

@Myoldmopar Myoldmopar merged commit 8a045ac into develop Jul 10, 2024
15 checks passed
@Myoldmopar Myoldmopar deleted the fixAirVolFlowStdDensity branch July 10, 2024 13:49
@yujiex
Copy link
Collaborator Author

yujiex commented Jul 10, 2024

Thanks @Myoldmopar and @mjwitte =)

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
8 participants