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

Address the water-source-heat-pump SHR greater than 1 issue #4897

Merged
merged 8 commits into from
May 11, 2015

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented May 5, 2015

This fix resolves the problem in bug issue #4567 . This bug issue is ready for reviewing and merging into develop. Pivotal tracker ID is 93830606. PR #4836 was an initial attempt to fix this bug, but the scope of that grew beyond this specific problem to a large question about sizing conditions. That broader issue remains open for further review. This fix narrows the scope to fixing only the sizing for Coil:Cooling:WaterToAirHeatPump:EquationFit in WaterToAirHeatPumpSimple.

@mjwitte
Copy link
Contributor

mjwitte commented May 6, 2015

@Nigusse So, if the entering and leaving humrat are the same, how is it possible that total is still < sensible in the HospitalLowEnergy? I know it's a small amount, but perhaps there should be a check to force them equal in this situation. See blelow, if we make sure the leaving humrat and temp are never higher than entering, this should go away.

I don't understand why you are touching FinalZoneSizing. This could have an unintended impact on other equipment in the same zone. I expected the change to be somewhere like line 1169, right before SupEnth is calculated. Both the humrat and drybulb temp should be reset (in my mind, anyway, it seems only logical)
SupTemp = min(SupTemp, MixTemp)
SupHumRat min(SupHumRat, MixHumRat)
This is a cooling device, so the capacity calcs should not calculate enthalpies that include any heating and/or humidification. This should be added in every block that's calculating a cooling capacity. Looks like there are four blocks of this 2 for total and 2 for sensible.

Then the blocks with the hard values of 48000 and 24 with this comment above them can be reduced to just the cap calc:
// This test avoids a negative capacity until a solution can be found.
This all happens multiple times in this routine.
@rraustad Would you agree?

@rraustad
Copy link
Contributor

rraustad commented May 6, 2015

I agree, do not write to FinalSysSizing in the component model. As Mike suggests, add a test to cap the supply temp/humrat at the appropriate places.

From what I see, add this at line 1089, 1149 and 1262. Include a comment so we know why this is there although it's rather obvious.

SupTemp = min(SupTemp, MixTemp)
SupHumRat = min(SupHumRat, MixHumRat)

What Mike is saying about these type of code:
if ( MixEnth > SupEnth ) {
CoolCapAtPeak = rhoair * VolFlowRate * ( MixEnth - SupEnth );
} else {
CoolCapAtPeak = rhoair * VolFlowRate * ( 48000.0 - SupEnth );
}

Is that they should be changed to just this:
CoolCapAtPeak = rhoair * VolFlowRate * ( MixEnth - SupEnth );

@Nigusse
Copy link
Contributor Author

Nigusse commented May 6, 2015

@mjwitte "So, if the entering and leaving humrat are the same, how is it possible that total is still < sensible in the HospitalLowEnergy?" As you said the diffs are small but there are two reasons for this diffs: (1) the sensible capacity is calculated using m.cp.dT approach and the total is based on m.dH; (2) there correction applied TotCapTempModFac and SensCapTempModFac come from different performance curves and can be slightly different. I think calculating using enthalpy difference may further minimize the diffs but capacity correction can still cause diffs. Are you suggesting that we force them to be equal?

@mjwitte "This should be added in every block that's calculating a cooling capacity. Looks like there are four blocks of this 2 for total and 2 for sensible."

I was only resolving the issue in ZoneHVAC:WaterToAirHeatPump object but not the unitary equipment used in air loop. The defect file involves zone equipment only.

@mjwitte @rraustad Ok. I will avoid changes to the FinalZoneSizing variable, instead make changes to the sizing calculation steps only as suggested.

@Nigusse
Copy link
Contributor Author

Nigusse commented May 6, 2015

Summary of changes made:
(1) Values in FinalZoneSizing variable will not change
(2) Added the following code in four places (sensible and total capacity for zone air loop )
SupTemp = min(SupTemp, MixTemp)
SupHumRat = min(SupHumRat, MixHumRat)
(3) The sensible and latent capacities calculation uses "m.dH" as shown below:
CoolCapAtPeak = rhoair * VolFlowRate * ( MixEnth - SupEnth );
MixEnth = PsyHFnTdbW( MixTemp, MixHumRat );
SupEnth = PsyHFnTdbW( SupTemp, MixHumRat );
theses enthalpies can also be calculated using SupHumRat, instead.
(4) checks if total < sensible, then set total = sensible when both are auto-sized.
(5) moved around the sizing reporting after the end of total and sensible sizing calculation

@Nigusse
Copy link
Contributor Author

Nigusse commented May 6, 2015

@mjwitte @rraustad I pushed the modified code and will review the results when the CI machine run is complete.

@Nigusse
Copy link
Contributor Author

Nigusse commented May 6, 2015

I think the unit test will not pass. I will modify it to check that total >= sensible, instead.

@@ -1086,6 +1086,9 @@ namespace WaterToAirHeatPumpSimple {
MixHumRat = OutAirFrac * FinalSysSizing( CurSysNum ).PrecoolHumRat + ( 1.0 - OutAirFrac ) * FinalSysSizing( CurSysNum ).RetHumRatAtCoolPeak;
}
}
// supply air condition is capped with that of mixed air to avoid SHR > 1.0
SupTemp = min( MixTemp, SupTemp );
SupHumRat = min( MixHumRat, SupHumRat );
OutTemp = FinalSysSizing( CurSysNum ).OutTempAtCoolPeak;
rhoair = PsyRhoAirFnPbTdbW( StdBaroPress, MixTemp, MixHumRat, RoutineName );
MixEnth = PsyHFnTdbW( MixTemp, MixHumRat );
Copy link
Contributor

Choose a reason for hiding this comment

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

@Myoldmopar Looks like we have some space vs tab issue going on here as well. See next 8 lines or so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte @Myoldmopar Is there an easy way to check space? I have to commit a modified unit test for this issue, so I can clean it before I re-commit the unit test update. I manually checked for space where I made changes but couldn't find one. Am I missing some thing here?

Copy link
Member

Choose a reason for hiding this comment

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

@Nigusse Sorry I didn't respond to this, but for reference, in my editor I have whitespace always shown. That helps me see if it accidentally put spaces instead of tabs. But, if the tab length is specified at 4, it still looks right. GitHub by default uses a tab length of 8, which makes it immediately clear when lines simply don't line up anymore.

@@ -1352,6 +1303,40 @@ namespace WaterToAirHeatPumpSimple {
if ( RatedCapCoolSensDes < SmallLoad ) {
RatedCapCoolSensDes = 0.0;
}
if ( RatedCapCoolTotalAutoSized && RatedCapCoolSensAutoSized ) {
if ( RatedCapCoolSensDes > RatedCapCoolTotalDes ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the autosized test be changed to "or" so if either is autosized this test is performed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am comparing auto-sized sensible and total capacities, so both should be auto-sized. if ( RatedCapCoolSensDes > RatedCapCoolTotalDes ).

@rraustad
Copy link
Contributor

rraustad commented May 8, 2015

Very moderate diff's for this change. I expect things are better now than before. Mike, can this be verified before next Tue? The changes Bereket describes seem reasonable to me.

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label May 11, 2015
mjwitte added a commit that referenced this pull request May 11, 2015
@mjwitte mjwitte merged commit 8cba006 into develop May 11, 2015
@mjwitte mjwitte deleted the WSHP_SHR_GreaterThan_1 branch May 11, 2015 21:21
@Myoldmopar Myoldmopar changed the title Wshp shr greater than 1 Address the water-source-heat-pump SHR greater than 1 issue May 19, 2015
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.

5 participants