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 Fan:ZoneExhaust AFN validation #10661

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Conversation

lymereJ
Copy link
Collaborator

@lymereJ lymereJ commented Aug 14, 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

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

@lymereJ lymereJ added Defect Includes code to repair a defect in EnergyPlus AirflowNetwork Related primarily on airflow-network portions of the codebase labels Aug 14, 2024
@lymereJ lymereJ added this to the EnergyPlus 24.2 milestone Aug 14, 2024
Copy link
Collaborator Author

@lymereJ lymereJ left a comment

Choose a reason for hiding this comment

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

Code walkthrough.

Comment on lines +11228 to +11236
}
if (!found) {
ShowSevereError(m_state, format("{}Fan:ZoneExhaust is not defined in {}", RoutineName, CurrentModuleObject));
ShowContinueError(
m_state,
format("The inlet node of the {} Fan:ZoneExhaust is not defined in the {}'s ZoneHVAC:EquipmentConnections",
m_state.dataZoneEquip->ZoneEquipList(j).EquipName,
m_state.dataZoneEquip->ZoneEquipConfig(j).ZoneName));
ErrorsFound = true;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved this verification so it can be done after iterating through all exhaust nodes.

Copy link
Member

Choose a reason for hiding this comment

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

👍 reasonable!

Comment on lines +19767 to +19780
" ZoneHVAC:Dehumidifier:DX,",
" North Zone Dehumidifier, !- Name",
" always_avail, !- Availability Schedule Name",
" Zone3DehumidifierInlet, !- Air Inlet Node Name",
" Dehumidifier Outlet Node,!- Air Outlet Node Name",
" 50.16, !- Rated Water Removal {L/day}",
" 3.412, !- Rated Energy Factor {L/kWh}",
" 0.12036, !- Rated Air Flow Rate {m3/s}",
" ZoneDehumidWaterRemoval, !- Water Removal Curve Name",
" ZoneDehumidEnergyFactor, !- Energy Factor Curve Name",
" ZoneDehumidPLFFPLR, !- Part Load Fraction Correlation Curve Name",
" 10.0, !- Minimum Dry-Bulb Temperature for Dehumidifier Operation {C}",
" 32.0, !- Maximum Dry-Bulb Temperature for Dehumidifier Operation {C}",
" 0.0; !- Off-Cycle Parasitic Electric Load {W}",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Add a dehumidifier to this test to create the same situation as the defect file (multiple exhaust nodes). The defect file uses a VRF terminal.

Copy link
Member

Choose a reason for hiding this comment

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

Nice job just modifying an existing test here.

Comment on lines +19830 to +19835

// Check that the validation fails if the AFN exhaust fan is not well setup
int exhaustFanInletNodeIndex = state->afn->MultizoneCompExhaustFanData(1).InletNode;
state->afn->MultizoneCompExhaustFanData(1).InletNode = 6;
state->afn->ValidateExhaustFanInputOneTimeFlag = true;
EXPECT_THROW(state->afn->validate_exhaust_fan_input(), std::runtime_error);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mess with the exhaust fan node to make sure that the if statement that was moved still creates a fatal error.

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.

I'm happy with this, any objections?

Comment on lines +11228 to +11236
}
if (!found) {
ShowSevereError(m_state, format("{}Fan:ZoneExhaust is not defined in {}", RoutineName, CurrentModuleObject));
ShowContinueError(
m_state,
format("The inlet node of the {} Fan:ZoneExhaust is not defined in the {}'s ZoneHVAC:EquipmentConnections",
m_state.dataZoneEquip->ZoneEquipList(j).EquipName,
m_state.dataZoneEquip->ZoneEquipConfig(j).ZoneName));
ErrorsFound = true;
Copy link
Member

Choose a reason for hiding this comment

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

👍 reasonable!

Comment on lines +19767 to +19780
" ZoneHVAC:Dehumidifier:DX,",
" North Zone Dehumidifier, !- Name",
" always_avail, !- Availability Schedule Name",
" Zone3DehumidifierInlet, !- Air Inlet Node Name",
" Dehumidifier Outlet Node,!- Air Outlet Node Name",
" 50.16, !- Rated Water Removal {L/day}",
" 3.412, !- Rated Energy Factor {L/kWh}",
" 0.12036, !- Rated Air Flow Rate {m3/s}",
" ZoneDehumidWaterRemoval, !- Water Removal Curve Name",
" ZoneDehumidEnergyFactor, !- Energy Factor Curve Name",
" ZoneDehumidPLFFPLR, !- Part Load Fraction Correlation Curve Name",
" 10.0, !- Minimum Dry-Bulb Temperature for Dehumidifier Operation {C}",
" 32.0, !- Maximum Dry-Bulb Temperature for Dehumidifier Operation {C}",
" 0.0; !- Off-Cycle Parasitic Electric Load {W}",
Copy link
Member

Choose a reason for hiding this comment

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

Nice job just modifying an existing test here.

@Myoldmopar
Copy link
Member

No objections, and it's still clean when I pull in develop and test. Thanks @lymereJ, merging.

@Myoldmopar Myoldmopar merged commit 4887a4d into develop Sep 11, 2024
15 checks passed
@Myoldmopar Myoldmopar deleted the fix_afn_zone_exh_fan_check branch September 11, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AirflowNetwork Related primarily on airflow-network portions of the codebase Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fan:ZoneExhaust node validation issue with Airflow Network
7 participants