NREL / EnergyPlus

EnergyPlus™ is a whole building energy simulation program that engineers, architects, and researchers use to model both energy consumption and water use in buildings.
https://energyplus.net
Other
1.12k stars 388 forks source link

Heat Recovery with Chiller:Electric:EIR #5742

Open asparke2 opened 8 years ago

asparke2 commented 8 years ago

Issue overview

Trying to make heat recovery work with Chiller:Electric:EIR. I took the HeatRecoveryElectricChiller.idf example file and replaced the Chiller:Electric with a Chiller:Electric:EIR and I got the errors below. @Myoldmopar thinks these 'Undefined' node types indicate a bug since the topology of the loops did not change at all and the example file runs fine.

   ** Severe  ** Same component name and type has differing Node Names.
   **   ~~~   **    Component:    CHILLER:ELECTRIC:EIR, name=BIG CHILLER
   **   ~~~   **    Nodes, inlet: BIG CHILLER INLET NODE, outlet: BIG CHILLER OUTLET NODE
   **   ~~~   **  & Nodes, inlet: BIG CHILLER HEAT REC INLET NODE, outlet: BIG CHILLER HEAT REC OUTLET NODE
   **   ~~~   **    Node Types:   Chilled Water Nodes & UNDEFINED
   ** Severe  ** Same component name and type has differing Node Names.
   **   ~~~   **    Component:    CHILLER:ELECTRIC:EIR, name=BIG CHILLER
   **   ~~~   **    Nodes, inlet: BIG CHILLER CONDENSER INLET NODE, outlet: BIG CHILLER CONDENSER OUTLET NODE
   **   ~~~   **  & Nodes, inlet: BIG CHILLER HEAT REC INLET NODE, outlet: BIG CHILLER HEAT REC OUTLET NODE
   **   ~~~   **    Node Types:   Condenser Water Nodes & UNDEFINED
   ** Warning ** Potential Node Connection Error for object CHILLER:ELECTRIC:EIR, name=BIG CHILLER
   **   ~~~   **   Node Types are still UNDEFINED -- See Branch/Node Details file for further information
   **   ~~~   **   Inlet Node : BIG CHILLER HEAT REC INLET NODE
   **   ~~~   **   Outlet Node: BIG CHILLER HEAT REC OUTLET NODE
   ************* There was 1 node connection error noted.
   **  Fatal  ** Previous Conditions cause program termination.

The modified example file is here: HeatRecoveryElectricChillerEIR.idf.txt

Details

mjwitte commented 8 years ago

@asparke2 This is a case of a bad default in the chiller object, and/or some bad logic in the chiller input processing. The first error in the output gives a clue:

   ** Warning ** GetElectricEIRChillerInput: Chiller:Electric:EIR="BIG CHILLER"
   **   ~~~   ** Since Reference Heat Reclaim Volume Flow Rate = 0.0, heat recovery is inactive.
   **   ~~~   ** However, node names were specified for heat recovery inlet or outlet nodes.

In the defect file, the Design Heat Recovery Water Flow Rate is blank, and the default is zero. This field is used by the chiller to decide if heat recovery is active or not. Since it's zero, the chiller throws the above warning and then does not register the heat recovery connections. So, that's why the node connection error happened. If the Design Heat Recovery Water Flow Rate is set to autosize, it runs fine.

So, options to fix: a. Change the wording of the above warning to mention that this will cause node connection errors if the heat recovery nodes are on a branch. b. Throw the above warning but go ahead and register the heat recovery nodes anyway if they are non-blank. Which would cause a similar connection error if the user didn't really intend to have heat recovery and the chiller heat recovery side isn't connected. c. Add a new field to say, heat recovery active yes/no. d. ???

Myoldmopar commented 8 years ago

Ah, I just got there too. What about just having the chiller check whether the user entered heat recovery nodes yet a zero/blank flow.

if ( ( ElectricEIRChiller( EIRChillerNum ).DesignHeatRecVolFlowRate > 0.0 ) || ( ElectricEIRChiller( EIRChillerNum ).DesignHeatRecVolFlowRate == AutoSiz     e ) ) {
  // actually register the nodes and do stuff
} else {
  // check if the nodes are non-blank, and give a little warning to indicate they were ignored
}
Myoldmopar commented 8 years ago

Of course, that's already there. Gya. I'm off this morning. Nevermind. If I had read through the error file slower, or through your message......

OK, I suggest we don't make a change here. The error file was already fine.

Myoldmopar commented 8 years ago

Per @asparke2, a similar change to (a) would be to change the warning to a severe. If the condition is going to cause a severe/fatal downstream when the branch/node manager tries to validate the compsets, then it would be more informative to just do it in the chiller model itself when it is validating its inputs. What do you think @mjwitte ?

mjwitte commented 8 years ago

@Myoldmopar That's reasonable, or, change the rule to be IF(heatrecovery nodes are present) then heat recovery is active, regardless of what the flow rate is. At a minimum, if the same logic remains, yes a severe here is fine, but the warning should also use the current field name for "Design Heat Recovery Water Flow Rate".

EnergyArchmage commented 8 years ago

If I understand brand/node/compsets stuff (which is not assured), it won't always lead to a fatal. Only if there is also a Branch objects with these nodes and this device. If the user left the flow blank and had a couple dangling nodes in there but no branch associated, it would still run with HR ignored.

So then you'd have a severe but no fatal, which folks also don't like.

mjwitte commented 8 years ago

@EnergyArchmage correct on the node/compsets understanding above. I think @Myoldmopar was suggesting that the errorsfound flag would also be set which would lead to a fatal. You're right that the warning was originally intended for the case where these node names are dangling but never used on a branch, so it's fine. The compsets error message from this case could also be improved.

Myoldmopar commented 8 years ago

Yeah, if I change it to a severe, I would trigger the fatal at the end of the GetInput. I don't really like the idea of letting it run with dangling nodes, it seems like that should also be a fatal, but that may be a story for a different day.

EnergyArchmage commented 8 years ago

EH? this fatal is from dangling nodes and it is not running. The branch object created the danglers here, not the chiller. I meant dangling in the input -- if the nodes don't ever get setup they aren't in memory.

mjwitte commented 8 years ago

So, this would never have come up if the Design Heat Recovery Water Flow Rate = 0.00055 had been copied from the original Chiller:Electric object to the new Chiller:Electric:EIR object. Let's do the minimal thing and change the warning text to something like this:

** Warning ** GetElectricEIRChillerInput: Chiller:Electric:EIR="BIG CHILLER"
   **   ~~~   ** Heat recovery is inactive because the Design Heat Recovery Water Flow Rate = 0.0 which is the default if blank.
   **   ~~~   ** The chiller Heat Recovery Inlet Node Name and Heat Recovery Outlet Node Name will be ignored.
   **   ~~~   ** If these nodes are referenced on a Branch, a fatal error will result.

I presume that a similar warning happens in other chiller models that support heat recovery, so they should all be updated.

Myoldmopar commented 8 years ago

I just think this is something the chiller should be validating with certainty instead of letting it run where it may fatal or maybe not. If the user wants heat recovery, they need a non-zero heat recovery design flow. If the user doesn't want heat recovery, they shouldn't enter node names that won't ever be used.

Something like 1e82db2 should be very clear to the user:

   ** Severe  ** GetElectricEIRChillerInput: Chiller:Electric:EIR="BIG CHILLER"
   **   ~~~   ** Invalid input configuration for heat recovery operation.
   **   ~~~   ** Field: "Design Heat Recovery Water Flow Rate" was blank or zero,
   **   ~~~   ** however, heat recovery node names were entered.
   **   ~~~   ** Either use a non-zero value for the flow rate, or remove the node names.
   **  Fatal  ** Errors found in processing input for Chiller:Electric:EIR
mjwitte commented 8 years ago

@Myoldmopar I'm fine with that, too, but then we'll need to make a transition rule to delete the excess node names if present so that a transtioned file that used to run with warning does not fail.

Myoldmopar commented 8 years ago

I'm fine adding that rule, and spreading these across the other chillers, but I'm not going to push hard on the matter. It certainly is less left to just refine the warning message per your comment.

It feels like leaving it as a warning message leaves it a little spongy, whereas the severe is a cinder block. If you trip on the cinder block, it might hurt for a second, but you'll clearly know how to avoid the cinder block next time 👍 .

EnergyArchmage commented 8 years ago

I think there a lot places where input fields are ignored based on the values in other input fields. Throwing lots of fatals for such input is not going to improve usability in IMHO.

Myoldmopar commented 8 years ago

OK, like I said, I'm not going to push on this. Both @EnergyArchmage and @mjwitte appear satisfied with the warning message approach, so I'll do the above in the chillers.

mitchute commented 3 years ago

@EnergyArchmage it looks like you may have addressed this at least partially in a1b944553c97234540603e37740ba87670cc0adf, found in #5740. Can you confirm? If so, maybe we close this issue?