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.

Sign up for GitHub

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

Jump to bottom

New outputs for infiltration and ventilation #10940

Merged
merged 13 commits into from
Feb 21, 2025
Merged

New outputs for infiltration and ventilation #10940

merged 13 commits into from
Feb 21, 2025
+428 −210

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Feb 14, 2025 •
edited
Loading

Pull request overview

Infiltration Outdoor Density Volume Flow Rate
Infiltration Standard Density Air Change Rate
Infiltration Outdoor Density Air Change Rate
Zone Infiltration Outdoor Density Volume Flow Rate
Zone Infiltration Standard Density Air Change Rate
Zone Infiltration Outdoor Density Air Change Rate
Zone Ventilation Outdoor Density Volume Flow Rate
Zone Ventilation Standard Density Air Change Rate
Zone Ventilation Outdoor Density Air Change Rate
  • Changes three existing outputs for consitency:
Infiltration Air Change Rate --> Infiltration Current Density Air Change Rate
Zone Infiltration Air Change Rate --> Zone Infiltration Current Density Air Change Rate
Zone Ventilation Air Change Rate --> Zone Ventilation Current Density Air Change Rate

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

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

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Feb 14, 2025
@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit 79b5c7f

Regression Summary
  • Audit: 437
  • RDD: 428
  • ESO Big Diffs: 8
  • ERR: 1
  • MTD: 1

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit f3ffc78

Regression Summary
  • Audit: 437
  • RDD: 428
  • ESO Small Diffs: 8
  • ERR: 1
  • MTD: 1

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit 8e1c25d

Regression Summary
  • Audit: 437
  • RDD: 428
  • ESO Small Diffs: 9

@mjwitte
Copy link
Contributor Author

mjwitte commented Feb 17, 2025

At this point diffs are as expected due to new output variables.

Regression Summary
Audit: 437
RDD: 428
ESO Small Diffs: 9

The ESO small diffs are due to a change in the order of calculations for air change rates.

@mjwitte mjwitte added the OutputChange Code changes output in such a way that it cannot be merged after IO freeze label Feb 17, 2025
@mjwitte
Copy link
Contributor Author

mjwitte commented Feb 17, 2025

Here's a test file for infiltration. All instances use coefficients of 1,0,0,0 to get a constant volume flow rate at the specified density basis. The density basis has been added to the name of each infiltration object "IN" "STD" or "OUT". The first five are specified with flow/zone, the last two with ACH.
image

Note that the ones based on Indoor density show some slight wiggle in the outputs due to the indoor density being slightly different when the mass flow rate is calculated in ZoneEquipmentManager::CalcAirFlowSimple and when it's converted back to volume flow rate in HVACManager::ReportInfiltrations.
5ZoneAirCooled-InfilDensityOptions.zip

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit f546f22

Regression Summary
  • Audit: 438
  • RDD: 428
  • ERR: 2
  • MTD: 2

@mjwitte
Copy link
Contributor Author

mjwitte commented Feb 17, 2025

Similar test file for ZoneVentilation. Note that ventilation does not have outputs per ZoneVentilation object, just at the zone level.
image

5ZoneNightVent2-VentDensityOptions.zip

@mjwitte mjwitte marked this pull request as ready for review February 17, 2025 21:14
mjwitte
mjwitte commented Feb 17, 2025
View reviewed changes
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.
This still needs a new or extended unit test, but otherwise it's ready for review.

src/EnergyPlus/HVACManager.cc
Comment on lines +2256 to +2260
if (vol > 0.0) {
Real64 secInHrOverVol = Constant::rSecsInHour / vol;
thisInfiltration.InfilAirChangeRateCurDensity = thisInfiltration.InfilVdotCurDensity * secInHrOverVol;
thisInfiltration.InfilAirChangeRateStdDensity = thisInfiltration.InfilVdotStdDensity * secInHrOverVol;
thisInfiltration.InfilAirChangeRateOutDensity = thisInfiltration.InfilVdotOutDensity * secInHrOverVol;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidate ACH calcs here. precalculating secInHrOverVol introduces some tiny diffs.

Copy link
Member

Choose a reason for hiding this comment

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

No problem.

src/EnergyPlus/HeatBalanceAirManager.cc
@@ -911,10 +911,8 @@ void GetSimpleAirModelInputs(EnergyPlusData &state, bool &ErrorsFound) // IF err
ShowContinueError(state, "Infiltration Coefficients are all zero. No Infiltration will be reported.");
}
}
if (!lAlphaFieldBlanks(5)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Address earlier comment from @rraustad

src/EnergyPlus/HeatBalanceAirManager.cc
Comment on lines +1146 to +1147
SetupOutputVariable(state,
"Infiltration Outdoor Density Volume Flow Rate",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add new output variables.

src/Transition/SupportFiles/Report Variables 24-2-0 to 25-1-0.csv
Zone Hybrid Unitary HVAC DehumidificationLoad to Humidistat Setpoint Heat Tansfer Energy,Zone Hybrid Unitary HVAC Dehumidification Load to Humidistat Setpoint Heat Transfer Energy,
Zone Hybrid Unitary HVAC Humidification Load to Humidistat Setpoint Heat Tansfer Energy,Zone Hybrid Unitary HVAC Humidification Load to Humidistat Setpoint Heat Transfer Energy,
Infiltration Air Change Rate,Infiltration Current Density Air Change Rate,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

3 existing output variables changed.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit cf8d6a8

Regression Summary
  • Audit: 437
  • RDD: 428

@mjwitte
Copy link
Contributor Author

mjwitte commented Feb 18, 2025

Transition tests.
5ZoneVAV-ChilledWaterStorage-Mixed
image

Exercise2 (modified)
image

FanCoil_HybridVent_VentSch
image

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit 6c7652b

Regression Summary
  • Audit: 437
  • RDD: 428

@github-actions GitHub Actions
Copy link

⚠️ Regressions detected on macos-14 for commit b038811

Regression Summary
  • Audit: 437
  • RDD: 428

@mjwitte mjwitte requested a review from shorowit February 18, 2025 17:53
Myoldmopar
Myoldmopar approved these changes Feb 21, 2025
View reviewed changes
Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

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

Good to go.

src/Transition/OutputRulesFiles/OutputChanges24-2-0-to-25-1-0.md
Zone Infiltration Outdoor Density Air Change Rate
Zone Ventilation Outdoor Density Volume Flow Rate
Zone Ventilation Standard Density Air Change Rate
Zone Ventilation Outdoor Density Air Change Rate
Copy link
Member

Choose a reason for hiding this comment

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

👍

src/Transition/SupportFiles/Report Variables 24-2-0 to 25-1-0.csv
Zone Hybrid Unitary HVAC DehumidificationLoad to Humidistat Setpoint Heat Tansfer Energy,Zone Hybrid Unitary HVAC Dehumidification Load to Humidistat Setpoint Heat Transfer Energy,
Zone Hybrid Unitary HVAC Humidification Load to Humidistat Setpoint Heat Tansfer Energy,Zone Hybrid Unitary HVAC Humidification Load to Humidistat Setpoint Heat Transfer Energy,
Infiltration Air Change Rate,Infiltration Current Density Air Change Rate,
Copy link
Member

Choose a reason for hiding this comment

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

👍

src/EnergyPlus/HVACManager.cc
Comment on lines +2256 to +2260
if (vol > 0.0) {
Real64 secInHrOverVol = Constant::rSecsInHour / vol;
thisInfiltration.InfilAirChangeRateCurDensity = thisInfiltration.InfilVdotCurDensity * secInHrOverVol;
thisInfiltration.InfilAirChangeRateStdDensity = thisInfiltration.InfilVdotStdDensity * secInHrOverVol;
thisInfiltration.InfilAirChangeRateOutDensity = thisInfiltration.InfilVdotOutDensity * secInHrOverVol;
Copy link
Member

Choose a reason for hiding this comment

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

No problem.

@Myoldmopar
Copy link
Member

This is good, the diffs are acceptable. Thanks for this cleanup @mjwitte , merging.

@Myoldmopar Myoldmopar merged commit 6ba97c9 into develop Feb 21, 2025
9 checks passed
@Myoldmopar Myoldmopar deleted the 8673InfilVentOutputs branch February 21, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rraustad rraustad rraustad left review comments

@Myoldmopar Myoldmopar Myoldmopar approved these changes

@shorowit shorowit Awaiting requested review from shorowit

Assignees
No one assigned
Labels
Defect Includes code to repair a defect in EnergyPlus OutputChange Code changes output in such a way that it cannot be merged after IO freeze
Projects
None yet
Milestone
EnergyPlus 25.1 IO Freeze
Development

Successfully merging this pull request may close these issues.

Possible bug in infiltration reporting
5 participants
@mjwitte @Myoldmopar @rraustad @nrel-bot-2 @nrel-bot-2c