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

Corrected CapFTemp curve index used in multi-speed DX coils for capacity sizing #7626

Merged
merged 2 commits into from
Jan 8, 2020

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented Nov 19, 2019

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)
  • 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

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

@Nigusse Nigusse added the Defect Includes code to repair a defect in EnergyPlus label Nov 19, 2019
@@ -14876,7 +14876,7 @@ namespace DXCoils {
(SELECT_CASE_var == CoilDX_HeatingEmpirical) || (SELECT_CASE_var == CoilDX_CoolingTwoStageWHumControl)) {
CapFTCurveIndex = DXCoil(CoilIndex).CCapFTemp(1);
} else if ((SELECT_CASE_var == CoilDX_MultiSpeedCooling) || (SELECT_CASE_var == CoilDX_MultiSpeedHeating)) {
CapFTCurveIndex = DXCoil(CoilIndex).MSCCapFTemp(1);
CapFTCurveIndex = DXCoil(CoilIndex).MSCCapFTemp(DXCoil(CoilIndex).NumOfSpeeds);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DX coils use the maximum speed for rated design sizing. Thus the curve index used for rated design capacity adjustment should point to the maximum speed curve, instead of the lowest speed.

Copy link
Member

Choose a reason for hiding this comment

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

An obvious fix, great.

@nrel-bot-3
Copy link

@Nigusse @lgentile it has been 28 days since this pull request was last updated.

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.

This is quite good to go. CI is fully green, the fix is obvious, and the unit test captures the behavior. Nice @Nigusse, thank you.

@@ -14876,7 +14876,7 @@ namespace DXCoils {
(SELECT_CASE_var == CoilDX_HeatingEmpirical) || (SELECT_CASE_var == CoilDX_CoolingTwoStageWHumControl)) {
CapFTCurveIndex = DXCoil(CoilIndex).CCapFTemp(1);
} else if ((SELECT_CASE_var == CoilDX_MultiSpeedCooling) || (SELECT_CASE_var == CoilDX_MultiSpeedHeating)) {
CapFTCurveIndex = DXCoil(CoilIndex).MSCCapFTemp(1);
CapFTCurveIndex = DXCoil(CoilIndex).MSCCapFTemp(DXCoil(CoilIndex).NumOfSpeeds);
Copy link
Member

Choose a reason for hiding this comment

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

An obvious fix, great.

Real64 PeakCoilCoolingLoad = 10000.0;
Real64 NominalCoolingDesignCapacity_lowestSpeed = PeakCoilCoolingLoad / TotCapTempModFac_lowestSpeed;
Real64 NominalCoolingDesignCapacity_designSpeed = PeakCoilCoolingLoad / TotCapTempModFac_designSpeed;
EXPECT_DOUBLE_EQ(9520.5999254239905, NominalCoolingDesignCapacity_lowestSpeed);
Copy link
Member

Choose a reason for hiding this comment

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

Unit test that exercises the results. Awesome.

@Myoldmopar Myoldmopar merged commit e217f0e into develop Jan 8, 2020
@Myoldmopar Myoldmopar deleted the 169848630_Issue7625 branch January 8, 2020 18:01
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.

CapFTCurveIndex is always set to speed 1 for multi-speed DX coils design size
6 participants