Variable speed coils can have higher flow rate than Packaged Terminal unit and fan #5723

nealkruis · 2016-06-10T22:46:23Z

Pull request overview

Add exceptions for mass imbalances. Correct problems as found in CI, such as:

Variable speed coil parent objects setting flow rates greater than the fan maximum flow (see related Correct ZoneHVAC system air flow rate to be the same as variable speed DX coil air flow to eliminate flow imbalance warnings #5632)

Work Checklist

Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
- At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
  - Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description

Review Checklist

…e variables are set correctly. Make assertions into real, informative errors.

nealkruis · 2016-06-10T22:51:45Z

@rraustad @mjwitte @EnergyArchmage Here is the PR for the mass balance checks and the corresponding fixes. So far I've only taken care of the PTHP issue discussed in #5632, but I anticipate others may fall out after iterating with the CI.

I changed the assertions to fatal errors and added the tolerance as @mjwitte suggested. This should clear out some (hopefully most?) of the failed integration tests. I'll keep you posted if I discover anything new.

nealkruis · 2016-06-10T22:58:44Z

src/EnergyPlus/PackagedTerminalHeatPump.cc

 				PTUnit( PTUnitNum ).MSCoolingSpeedRatio( Iter ) = VarSpeedCoil( PTUnit( PTUnitNum ).DXCoolCoilIndexNum ).MSRatedAirVolFlowRate( Iter ) / VarSpeedCoil( PTUnit( PTUnitNum ).DXCoolCoilIndexNum ).MSRatedAirVolFlowRate( PTUnit( PTUnitNum ).NumOfSpeedCooling );
+				PTUnit( PTUnitNum ).CoolVolumeFlowRate( Iter ) = PTUnit( PTUnitNum ).MaxCoolAirVolFlow * PTUnit( PTUnitNum ).MSCoolingSpeedRatio( Iter );


@rraustad this is the change I ended up making. I believe this is in line with the changes you had in mind, but you might want to look them over to be sure.

This is what I was thinking, but it does scare me a bit. Off design flow rates only work up to a point.

rraustad · 2016-06-10T23:20:11Z

src/EnergyPlus/HVACManager.cc

@@ -515,6 +518,8 @@ namespace HVACManager {

 			ManageEMS( emsCallFromEndSystemTimestepBeforeHVACReporting, anyEMSRan ); // EMS calling point

+			CheckMassBalances();


So this is staying?

It shouldn't add any significant time to the simulation, and we should always be checking for mass conservation problems. There bay be a better time to check these. Ideas?

rraustad · 2016-06-11T00:19:00Z

I've tried several times and can't pull this branch:

fatal: Unable to create
'C:/Users/Richard/Documents/GitHub/EnergyPlusTeam/.git/refs/remotes/origin/121319197Replace-a-field-name-of-"Controlled-Zone-Name"-in-two-objects-#5664.lock': Invalid argument
Unexpected end of command stream

git did not exit cleanly (exit code 1) (4484 ms @ 6/10/2016 8:17:43 PM)

lgu1234 · 2016-06-11T00:50:04Z

@rraustad I got the same problem when I switched branch. The branch with the problem was created by me. It may contain not-allowed characters. If possible, you may close the branch. I don't know how to close it. I can create it late.

rraustad · 2016-06-11T01:59:09Z

Deleted that branch, now it's OK

…servation-tests

rraustad · 2016-06-11T02:38:45Z

src/EnergyPlus/HVACManager.cc

+
+			if (abs(Node( Fan( FanNum ).OutletNodeNum ).MassFlowRate - Fan( FanNum ).OutletAirMassFlowRate) > SmallMassFlow) {
+				ShowFatalError(Fan(FanNum).FanType + ", Name: " + Fan(FanNum).FanName + ": Fan outlet mass flow rate (" + RoundSigDigits(Fan( FanNum ).OutletAirMassFlowRate, 4) + ") does not match outlet node mass flow rate (" + RoundSigDigits(Node( Fan( FanNum ).OutletNodeNum ).MassFlowRate, 4) + ").");
+			}


I see no need for these. Either outlet = inlet or it doesn't.

… into conservation-tests

rraustad · 2016-06-11T03:20:39Z

These changes are fundamentally what I expected. The parent design flow can be different than the child rated flow. And if so, the child coil rides the capacity and EIR as a function of flow fraction curve. This is how it's supposed to work, but what worries me is the capacity/flow relationship. I guess we allow that for other coil types so let's see what happens here.

rraustad · 2016-06-11T03:34:32Z

So where's the CI, on vacation? I'm pushin up a change and we can argue what the end result should look like.

rraustad · 2016-06-11T03:46:17Z

src/EnergyPlus/PackagedTerminalHeatPump.cc

+			PTUnit( PTUnitNum ).MaxCoolAirMassFlow = RhoAir * PTUnit( PTUnitNum ).MaxCoolAirVolFlow;
+			PTUnit( PTUnitNum ).MaxHeatAirMassFlow = RhoAir * PTUnit( PTUnitNum ).MaxHeatAirVolFlow;
+
+			if ( PTUnit( PTUnitNum ).useVSCoilModel && PTUnit( PTUnitNum ).NumOfSpeedCooling == 0 ) {


So what is "PTUnit( PTUnitNum ).NumOfSpeedCooling == 0" supposed to mean here?

It must be NumOfSpeedCooling > 0 for multispeed, and = 0 for VS? Or that the speed variables have not be calculated yet?

Good question. This was already here when I found it. @EnergyArchmage?

yes, it is NumOfSpeedCooling > 0 for multispeed, and = 0 for VS. I think these have been moving to use the bool over time, moving away from == 0 for VS, but it still lingers.

rraustad · 2016-06-11T03:54:07Z

I've looked at this back and forth and don't see any issue with Neal's logic. Can't wait to see some results.

mjwitte · 2016-06-11T18:18:05Z

@rraustad Regarding CI, it's been messed up since sometime late Thursday. @Myoldmopar is working on it.

nealkruis · 2016-06-13T23:47:38Z

@rraustad @mjwitte

Results are coming back. There are 8 testfiles with mass balance problems (with links to the problem parent objects):

DirectIndirectEvapCoolersVSAS.idf (AirLoopHVAC:Unitary)
FanCoil_HybridVent_VentSch.idf (ZoneHVAC:FourPipeFanCoil)
HVACTemplate-5ZoneVAVFanPowered.idf (HVACTemplate:Zone:VAV:FanPowered)
A pump with round-off error (need to change the tolerance on the water mass balance check).
StripMallZoneEvapCooler.idf (ZoneHVAC:EvaporativeCoolerUnit)
StripMallZoneEvapCoolerAutosized.idf (ZoneHVAC:EvaporativeCoolerUnit)
WaterHeaterHeatPumpWrappedCondenser.idf (WaterHeater:HeatPump:WrappedCondenser)
_FanCoilHybridVentAFN.idf (ZoneHVAC:FourPipeFanCoil)

mjwitte · 2016-09-06T13:36:24Z

@nealkruis @rraustad What is the status on this PR? Is it ready for final review and merge? Since the mass balance checks were moved out of here, the overview at the top needs to be revised.

rraustad · 2016-09-06T14:41:35Z

Not sure what to do with this one. I believe the change made to the VS DX coil altered the model where the Field: Nominal Speed Level is no longer used as intended (see Jul 18 comment above).

nealkruis · 2016-09-07T18:39:55Z

@rraustad The Nominal Speed Level is still used to determine the actual capacities and air flow rates of the various speeds from the catalogue data (see the VS coil docs). This happens in VariableSpeedCoils.cc when the input is processed. In my understanding, the nominal rating values shouldn't be used elsewhere in the simulation (e.g., PackagedTerminalHeatPump.cc, or HVACUnitarySystem.cc).

@mjwitte I think the same change needs to happen in any parent object that uses variable speed coils (both heating and cooling). I don't have time immediately to look into this, but perhaps someone else can?

nrel-bot-3 · 2016-10-06T00:25:26Z