-
Notifications
You must be signed in to change notification settings - Fork 397
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 #10355 - Correctly Set up Zone Internal Gains when Refrigeration:CompressorRack serves a Refrigeration:WalkIn object with "Zone" Heat Transfer Location #10525
Conversation
…CompressorRack serves a Refrigeration:WalkIn object with "Zone" Heat Transfer Location
@@ -197,6 +197,7 @@ set(test_src | |||
Psychrometrics.unit.cc | |||
Pumps.unit.cc | |||
PurchasedAirManager.unit.cc | |||
RefrigeratedCase.unit.cc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite scary that there was zero unit test for any Refrigeration objects...
; !- Zone Equipment Sequential Heating Fraction Schedule Name 1 | ||
)IDF"; | ||
|
||
TEST_F(EnergyPlusFixture, RefrigeratedRackWithCaseInZone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I've set up 4 unit tests:
- CompressorRack references a Case, directly
- CompressorRack references a WalkIn, directly (failed initially)
- CompressorRack references only a WalkIn, via a CaseAndWalkInList (failed initially)
- CompressorRack references both a Walkin and a Case, via a CaseAndWalkinList (Walkin first, then case).
// If CoilFlag is true and Location is Zone, GetRefrigerationInput ensures you must have a Heat Rejection Zone provided already | ||
SetupZoneInternalGain(state, | ||
RefrigCase(rack.CaseNum(1)).ActualZoneNum, | ||
rack.HeatRejectionZoneNum, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case that means NumCoils > 0, Get input routine will throw if you don't have a HeatRejectionZone, so safe to use
|
||
// if Location is Zone, GetRefrigerationInputEither checks that you have at least one load and that either: | ||
// * You have only cases, and they must be all in the same zone | ||
// * Or you must have a Heat Rejection Zone provided | ||
int rackZoneNum = -1; | ||
if (rack.HeatRejectionZoneNum > 0) { | ||
rackZoneNum = rack.HeatRejectionZoneNum; | ||
} else { | ||
rackZoneNum = RefrigCase(rack.CaseNum(1)).ActualZoneNum; | ||
} | ||
SetupZoneInternalGain(state, | ||
RefrigCase(rack.CaseNum(1)).ActualZoneNum, | ||
rackZoneNum, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When CoilFlag is not set, NumCoils == 0, and either you have NumWalkIns > 0, in which case you MUST have a heat rejection zone. Otherwise that means NumCases > 0 and it's enforced that all of them must be in the same zone.
(It also enforces that NumCoils + NumWalkIns + NumCases > 0, so we know there is a load)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good here. Thanks @jmarrec !
Pull request overview
I have ran the Defect file (which I hosted on EnergyPlusDevSupport/DefectFiles/10000s/10355) before and after fix. Confirmed the segfault before, and completion after.
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.