Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Fix #6204 - Coil:Cooling:DX:VariableSpeed air-to-air coil should autosize cooling capacity using CapFT curve results based on OutTemp and not RatedInletWaterTemp #7996

Merged
merged 5 commits into from
Jun 3, 2020
Merged

Fix #6204 - Coil:Cooling:DX:VariableSpeed air-to-air coil should autosize cooling capacity using CapFT curve results based on OutTemp and not RatedInletWaterTemp #7996

merged 5 commits into from
Jun 3, 2020

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented May 18, 2020 •
edited
Loading

Pull request overview

I modified an existing unit test ZoneUnitarySysTest.UnitarySystemModel_MultispeedPerformance that had a workaround in place which was to make the DataSizing::DesDayWeath(1).Temp(1) = 29.4; the same as the RatedInletWaterTemp (which @rraustad did when he found the issue and reported it)

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

@jmarrec jmarrec added the Defect Includes code to repair a defect in EnergyPlus label May 18, 2020
@jmarrec jmarrec self-assigned this May 18, 2020
jmarrec
jmarrec commented May 18, 2020
View reviewed changes
tst/EnergyPlus/unit/UnitarySystem.unit.cc
Comment on lines +2433 to +2434
Real64 RatedSourceTempCool = DataSizing::DesDayWeath(1).Temp(1);
EXPECT_EQ(RatedSourceTempCool, 35.0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once I remove the workaround, this test now fails.

src/EnergyPlus/VariableSpeedCoils.cc
Comment on lines +4084 to +4090
if (VarSpeedCoil(DXCoilNum).CondenserInletNodeNum != 0) {
RatedSourceTempCool = RatedInletWaterTemp;
} else {
RatedSourceTempCool = OutTemp;
}
TotCapTempModFac =
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedInletWaterTemp);
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedSourceTempCool);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix 1 out of 2

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

src/EnergyPlus/VariableSpeedCoils.cc
Comment on lines +4161 to +4167
if (VarSpeedCoil(DXCoilNum).CondenserInletNodeNum != 0) {
RatedSourceTempCool = RatedInletWaterTemp;
} else {
RatedSourceTempCool = OutTemp;
}
TotCapTempModFac =
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedInletWaterTemp);
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedSourceTempCool);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix 2 out of 2

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

jmarrec
jmarrec commented May 18, 2020
View reviewed changes
tst/EnergyPlus/unit/UnitarySystem.unit.cc
Comment on lines -2313 to -2672
// overwrite outdoor weather temp to variable speed coil rated water temp until this gets fixed
DataSizing::DesDayWeath(1).Temp(1) = 29.4;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old workaround.

tst/EnergyPlus/unit/UnitarySystem.unit.cc
Comment on lines +2436 to +2446
Real64 TotCapTempModFac = CurveManager::CurveValue(VariableSpeedCoils::VarSpeedCoil(1).MSCCapFTemp(VariableSpeedCoils::VarSpeedCoil(1).NormSpedLevel),
17.410329442560833,
RatedSourceTempCool);

EXPECT_NEAR(TotCapTempModFac, 0.930018048445091, 0.001);
Real64 RatedCapCoolTotalDes = CoolCoolCapAtPeak / TotCapTempModFac;
EXPECT_NEAR(RatedCapCoolTotalDes, 34897.0396944, 0.001);

EXPECT_NEAR(thisSys->m_DesignCoolingCapacity, RatedCapCoolTotalDes, 0.001);
EXPECT_EQ(thisSys->m_DesignCoolingCapacity, VariableSpeedCoils::VarSpeedCoil(1).RatedCapCoolTotal);
EXPECT_NEAR(thisSys->m_DesignHeatingCapacity, 32752.541, 0.001);
EXPECT_NEAR(thisSys->m_DesignHeatingCapacity, RatedCapCoolTotalDes, 0.001);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated values due to using the right OutTemp for the curve evaluation

@mitchute mitchute added this to the EnergyPlus 9.4.0 milestone Jun 2, 2020
mitchute
mitchute approved these changes Jun 3, 2020
View reviewed changes
Copy link
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

Looks good. This should be able to merge shortly.

src/EnergyPlus/VariableSpeedCoils.cc
Comment on lines +4084 to +4090
if (VarSpeedCoil(DXCoilNum).CondenserInletNodeNum != 0) {
RatedSourceTempCool = RatedInletWaterTemp;
} else {
RatedSourceTempCool = OutTemp;
}
TotCapTempModFac =
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedInletWaterTemp);
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedSourceTempCool);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

tst/EnergyPlus/unit/UnitarySystem.unit.cc
EXPECT_NEAR(VariableSpeedCoils::VarSpeedCoil(2).RatedAirVolFlowRate, 1.70, 0.01); // VS DX heating coil was not autosized

// checks on autosized cooling air flow rates
EXPECT_NEAR(thisSys->m_CoolVolumeFlowRate[1], 0.182964, 0.000001);
EXPECT_NEAR(thisSys->m_CoolVolumeFlowRate[1], fullFlow * 0.1, 0.000001);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nicer.

src/EnergyPlus/VariableSpeedCoils.cc
Comment on lines +4161 to +4167
if (VarSpeedCoil(DXCoilNum).CondenserInletNodeNum != 0) {
RatedSourceTempCool = RatedInletWaterTemp;
} else {
RatedSourceTempCool = OutTemp;
}
TotCapTempModFac =
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedInletWaterTemp);
CurveValue(VarSpeedCoil(DXCoilNum).MSCCapFTemp(VarSpeedCoil(DXCoilNum).NormSpedLevel), MixWetBulb, RatedSourceTempCool);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@mitchute
Copy link
Collaborator

mitchute commented Jun 3, 2020 •
edited
Loading

CI is generally happy aside from the results diffs, and they all appear to be justified due to the change to the sizing condition. Merging.

@mitchute mitchute merged commit d09b147 into develop Jun 3, 2020
@mitchute mitchute deleted the 6204_CoilCoolingDXVariableSpeed_AirToAir_CoolCapSizing branch June 3, 2020 14:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@mitchute mitchute mitchute approved these changes

Assignees

@jmarrec jmarrec

Labels
Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
Milestone
EnergyPlus 9.4.0
7 participants
@jmarrec @mitchute @nrel-bot @nrel-bot-2 @nrel-bot-3 @nrel-bot-2b @nrel-bot-2c