Skip to content

Conversation

tanaya-mankad
Copy link
Collaborator

Pull request overview

Description of the purpose of this PR

A client using a proprietary model in E+23.1 noticed that when the design day temperature was cooler than the cooling coil supply temperature, the simulation crashed. This behavior was inadvertently circumvented by a pre-condition check in the DXCoils CBF calculation, that exits early if airflow or total capacity are zero. However, the user is never warned that an input condition caused a negative total capacity to be calculated, which was then zeroed out for safety.
This PR creates a Severe Error to indicate the inverted temperature condition.

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

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

@tanaya-mankad tanaya-mankad self-assigned this Sep 5, 2025
@tanaya-mankad tanaya-mankad added the Defect Includes code to repair a defect in EnergyPlus label Sep 5, 2025
int constexpr SpeedCal{1};
VariableSpeedCoils::InitVarSpeedCoil(*state, DXCoilNum, SensLoad, LatentLoad, fanOp, OnOffAirFlowRatio, SpeedRatio, SpeedCal);
EXPECT_TRUE(compare_err_stream_substring("entering air temperature is less than design outlet air temperature", true));
EXPECT_TRUE(compare_err_stream_substring("This will yield negative coil capacity sizing", true));
Copy link
Contributor

Choose a reason for hiding this comment

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

This unit test should fail. The old message used "will" and the new warning message above uses:

This would yield negative coil capacity sizing

format("On design day {}, cooling coil entering air temperature {:.2R} is less than design supply air temperature {:.2R}. This would "
"yield negative coil capacity sizing for {}.", state.dataSize->FinalSysSizing(state.dataSize->CurSysNum).CoolDesDay, MixTemp, SupTemp, varSpeedCoil.Name));
ShowContinueError(state, "Cooling capacity is set to zero during sizing; simulation continues.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Coil sizing is based on enthalpy. Given the sneaky use of a standard capacity if enthalpies are not correct CoilCapAtPeak should never be negative here. I would like to see the 48000 removed (i.e., delete the else) since it just forces a non-zero coil capacity. There is nothing wrong with the coil sizing to 0 if FinalSysSizing conditions warrant that result.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes! Not knowing where it came from, I was reticent to touch a "magic number."

@tanaya-mankad tanaya-mankad changed the title Issue Severe Error on negative coil capacity sizing Issue Warning Error on negative coil capacity sizing Oct 2, 2025
@tanaya-mankad
Copy link
Collaborator Author

Our main energy modeler, Aaron, suggested that the client might like an even fuller description of the air state that causes a negative capacity, in the vein of this code, even though the eventual coil size = 0 will be valid.
The 48000 has been removed as well (which may cause regression failures, I'll see...)

@rraustad
Copy link
Contributor

rraustad commented Oct 8, 2025

@tanaya-mankad that would be a good fix for this issue.

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

Development

Successfully merging this pull request may close these issues.

Negative coil capacity when design day entering coil temp < supply air temp

4 participants