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

New feature - ASHRAE 90.1 SZVAV control for Unitary Systems #5994

Merged

Conversation

rraustad
Copy link
Contributor

@rraustad rraustad commented Jan 27, 2017

Pull request overview

ASHRAE Standard 90.1 requires low speed air flow when zone loads are 50% of design. AirloopHVAC:UnitarySystem now conforms to this specification. As zone loads increase, the fan speed increases while maintaining a user specified outlet air temperature. When fan speed reaches maximum speed, outlet air temperature is allowed to increase to maintain zone thermostat set point temperatures.

Work Checklist

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)
  • 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
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

  • Code style (parentheses padding, variable names)
  • Functional code review (it has to work!)
  • If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check -- verify this
  • Unit Test(s)
  • C++ checks:
    • Argument types
    • If any virtual classes, ensure virtual destructor included, other things
  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults
    • Open windows IDF Editor with modified IDD to check for errors
    • If transition, add rules to spreadsheet
    • If transition, add transition source
    • If transition, update idfs
  • If new idf included, locally check the err file and other outputs
  • Documentation changes in place
  • Changed docs build successfully
  • ExpandObjects changes?
  • If output changes, including tabular output structure, add to output rules file for interfaces

@rraustad
Copy link
Contributor Author

This appears to be working well although I still need to test other configurations and weather to be sure.

unitarysystem_szvav_watercoils

@mjwitte mjwitte added IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus labels Feb 13, 2017
@mjwitte mjwitte added this to the EnergyPlus 8.7.0 milestone Feb 13, 2017
@rraustad rraustad changed the title Branch pt133379791 new feature szvav code and unit tests New feature - ASHRAE 90.1 SZVAV control for Unitary Systems Feb 13, 2017
@@ -52999,9 +53001,13 @@ AirLoopHVAC:UnitarySystem,
\type choice
\key Load
\key SetPoint
\key ASHRAE90VariableFan
Copy link
Member

Choose a reason for hiding this comment

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

I won't hold anything up if this key has already been approved during calls, but I will say that, to me, ASHRAE90VariableFan doesn't have a good feel to it. Maybe SingleZoneVAV? SingleZoneVariable?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was discussed early on, but if it strikes you as odd now, then we should revisit it.

Copy link
Member

Choose a reason for hiding this comment

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

Pausing......I don't really care that much. I really don't want to make a big discussion, but it seems like the other two keys for that object are specific, clear control terms. This new key should also be a clear control term. Is this control mode something specific to only ASHRAE? Or is it just that ASHRAE requires it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The choice key matches what is used in fan coil. I don't mind changing this now since I have a few other details to correct. The term is meant to describe the ASHRAE 90.1 method of two- or variable-speed fans used with DX and water coils, and allowing low speed fan during low load conditions. If it were to change, I do like SingleZoneVAV although UnitarySystem has a multi-zone capability.

ZoneHVAC:FourPipeFanCoil,
 A3 , \field Capacity Control Method
    \required-field
    \type choice
    \key ConstantFanVariableFlow
    \key CyclingFan
    \key VariableFanVariableFlow
    \key VariableFanConstantFlow
    \key MultiSpeedFan
    \key ASHRAE90VariableFan

Copy link
Contributor

Choose a reason for hiding this comment

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

There are so many similar choices in the fan coil that ASHRAE90 is almost required to distinguish this from the other variable fan options. But for the UnitarySystem, I like SingleZoneVAV. The "Load" option works off the single control zone as well, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. Both SingleZoneVAV and Load require a control zone name input. I'll make the change.

@rraustad
Copy link
Contributor Author

Finally got the docs to build and will be uploading a new version with minor edits (e.g., get rid of "ASHRAE' in docs, reformat lists, etc.). I'll wait on comments before committing a new version.

@mjwitte mjwitte mentioned this pull request Feb 15, 2017
20 tasks
@rraustad
Copy link
Contributor Author

I am comfortable with what I see as diff's from the CI. The one big MTR diff I can't replicate since all files here show no diff's between develop and this branch. I still have one more check-in to update my changes to the docs but am waiting on comments before that happens.

* * * * * * * * * * * * 
HVACTemplate-5ZoneUnitarySystem.csv vs HVACTemplate-5ZoneUnitarySystem_new.csv
All Records are Equal.    48 records compared
* * * * * * * * * * * * 
UnitarySystem_5ZoneWaterLoopHeatPump.csv vs UnitarySystem_5ZoneWaterLoopHeatPump_new.csv
All Records are Equal.    48 records compared
* * * * * * * * * * * * 
UnitarySystem_DXCoilSystemAuto.csv vs UnitarySystem_DXCoilSystemAuto_new.csv
All Records are Equal.    48 records compared
* * * * * * * * * * * * 
UnitarySystem_FurnaceWithDXSystemRHcontrol.csv vs UnitarySystem_FurnaceWithDXSystemRHcontrol_new.csv
All Records are Equal.    480 records compared
* * * * * * * * * * * * 
UnitarySystem_HeatPumpAuto.csv vs UnitarySystem_HeatPumpAuto_new.csv
All Records are Equal.    48 records compared
* * * * * * * * * * * * 
UnitarySystem_MultiSpeedCoils_SingleMode.csv vs UnitarySystem_MultiSpeedCoils_SingleMode_new.csv
All Records are Equal.    576 records compared
* * * * * * * * * * * * 
UnitarySystem_VSHeatPumpWaterToAirEquationFit.csv vs UnitarySystem_VSHeatPumpWaterToAirEquationFit_new.csv
All Records are Equal.    1536 records compared
* * * * * * * * * * * * 
UnitarySystem_WaterCoils_wMultiSpeedFan.csv vs UnitarySystem_WaterCoils_wMultiSpeedFan_new.csv
All Records are Equal.    480 records compared
* * * * * * * * * * * * 

@rraustad
Copy link
Contributor Author

Considering this the final commit.

@@ -1479,54 +1414,6 @@ namespace HVACUnitarySystem {
}
}

// Find the number of zones (zone Inlet Nodes) attached to an air loop from the air loop number
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this code to sizing since I needed to know the control zone fraction in order to autosize the min/max temperature limits.

@@ -2221,7 +2110,25 @@ namespace HVACUnitarySystem {

SizingMethod = SystemAirflowSizing;

if ( UnitarySystem( UnitarySysNum ).NoCoolHeatSAFMethod == FractionOfAutoSizedCoolingValue ) {
if ( UnitarySystem( UnitarySysNum ).NoCoolHeatSAFMethod <= SupplyAirFlowRate && UnitarySystem( UnitarySysNum ).ControlType == CCM_ASHRAE ) {
SizingMethod = AutoCalculateSizing;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

autosizes no load air flow rate to 67% of max if using DX coils, otherwise 50%



if ( UnitarySystem( UnitarySysNum ).ControlType == CCM_ASHRAE ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

autosize no load air flow for single zone VAV model

@@ -3136,6 +3122,10 @@ namespace HVACUnitarySystem {
UnitarySystem( UnitarySysNum ).ControlType = LoadBased;
} else if ( SameString( Alphas( iControlTypeAlphaNum ), "SetPoint" ) ) {
UnitarySystem( UnitarySysNum ).ControlType = SetPointBased;
} else if ( SameString( Alphas( iControlTypeAlphaNum ), "ASHRAE90VariableFan" ) ) {
UnitarySystem( UnitarySysNum ).ControlType = CCM_ASHRAE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really! this needs to be changed to SingleZoneVAV

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean this still needs to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the key field from the idd. I'll test to make sure it works and check in a final today.

@@ -6133,6 +6140,52 @@ namespace HVACUnitarySystem {
}
}

// check for specific input requirements for ASHRAE90.1 model
if ( UnitarySystem( UnitarySysNum ).ControlType == CCM_ASHRAE ) {
Copy link
Contributor Author

@rraustad rraustad Feb 15, 2017

Choose a reason for hiding this comment

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

checking here for coils allowed for the SingleZoneVAV model.

TempMaxPLR = 1.0; // enforce a perfect 1.0 at the top end
// use the ASHRAE 90.1 method of reduced fan speed at low loads
if ( UnitarySystem( UnitarySysNum ).simASHRAEModel ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is the single zone VAV model

if ( UnitarySystem( UnitarySysNum ).simASHRAEModel ) {

// set up mode specific variables to use in common function calls
if ( CoolingLoad ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

set up locals based on cooling or heating load to be used in common calls

coilAirOutletNode = 0;
}
// set up RegulaFalsi variables
Par( 1 ) = double( UnitarySysNum );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

set up RegulaFalsi arguments and only change when needed in the model

// Step 9: not enough cooling or heating, increase coil capacity
// DONE
//
// Step 1: set min air flow and full coil capacity
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then follow Step x instructions

}
SetSpeedVariables( UnitarySysNum, true, TempMaxPLR );
CalcUnitarySystemToLoad( UnitarySysNum, AirLoopNum, FirstHVACIteration, TempMaxPLR, HeatPLR, OnOffAirFlowRatio, TempSensOutput, TempLatOutput, HXUnitOn, _, _, CompressorONFlag );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing deleted here, Git is just confused.

if( coilLoopNum > 0 ) SetComponentFlowRate( Node( coilFluidInletNode ).MassFlowRate, coilFluidInletNode, coilFluidOutletNode, coilLoopNum, coilLoopSide, coilBranchNum, coilCompNum );

} else { // not ASHRAE model

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if not single zone VAV, then do what we've always done.

@@ -6945,155 +6935,567 @@ namespace HVACUnitarySystem {
// if no load, or only a moisture load which can't be met at PLR=1, RETURN
return;
}
// must test to see if load is bounded by capacity before calling RegulaFalsi
if ( ( HeatingLoad && ZoneLoad < SensOutputOn ) || ( CoolingLoad && ZoneLoad > SensOutputOn ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These next pink code were not deleted, they are now part of he ELSE after the IF ASHRAE model code...see below at ESLE

TempMaxPLR += 0.1;
if ( TempMaxPLR > 0.95 && TempMaxPLR < 1.05 ) {
TempMaxPLR = 1.0; // enforce a perfect 1.0 at the top end
// use the ASHRAE 90.1 method of reduced fan speed at low loads
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This next line is the IF ASHRAE model

@@ -7258,6 +7660,172 @@ namespace HVACUnitarySystem {

}

Real64
CalcUnitarySystemWaterFlowResidual(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

new function to iterate on either air flow or coil capacity

@@ -8108,7 +8610,11 @@ namespace HVACUnitarySystem {
SimulateHeatingCoilComponents( CompName, FirstHVACIteration, _, 0, _, _, UnitarySystem( UnitarySysNum ).FanOpMode, PartLoadRatio, UnitarySystem( UnitarySysNum ).HeatingSpeedNum, UnitarySystem( UnitarySysNum ).HeatingSpeedRatio );
UnitarySystem( UnitarySysNum ).HeatingCycRatio = PartLoadRatio;
} else if ( SELECT_CASE_var == Coil_HeatingWater ) {
mdot = min( Node( UnitarySystem( UnitarySysNum ).HeatCoilFluidOutletNodeNum ).MassFlowRateMaxAvail, UnitarySystem( UnitarySysNum ).MaxHeatCoilFluidFlow * PartLoadRatio );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to trick the water coils to use a different PLR than the fan

// SUBROUTINE LOCAL VARIABLE DECLARATIONS:
int InletNode; // inlet node number
Real64 AverageUnitMassFlow; // average supply air mass flow rate over time step
int SpeedNum; // speed for multi-speed or variable-speed coils
bool FanOn;
Real64 fanPartLoadRatio; // local variable for PLR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here I introduced a local fanPLR variable so I could bait and switch when the ASHRAE model was active. The fan PLR and coil PLR are independent. Maybe a better way to do this, but this is what's here now.

@@ -1238,6 +1238,22 @@ namespace ReportSizingManager {
ShowContinueError( "SizingString = " + SizingString + ", DataCapacityUsedForSizing = " + TrimSigDigits( DataCapacityUsedForSizing, 1 ) );
ShowContinueError( "SizingString = " + SizingString + ", DataFlowUsedForSizing = " + TrimSigDigits( DataFlowUsedForSizing, 1 ) );
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New function to calculate SAT based on zone temp and not coil inlet temp. Using zone temp represents SAT needed to meet a zone load, using coil inlet temp represents SAT needed to meet a system capacity. Subtle difference.

@@ -344,29 +345,12 @@ namespace HVACUnitarySystem {
// PURPOSE OF THIS SUBROUTINE:
// This subroutine manages unitary system component simulation.

// METHODOLOGY EMPLOYED:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted lots of these unnecessary comments, blow right by them when reviewing.

@@ -437,7 +421,7 @@ namespace HVACUnitarySystem {
} else {
ControlUnitarySystemtoSP( UnitarySysNum, AirLoopNum, FirstHVACIteration, CompOn, OAUCoilOutTemp, HXUnitOn );
}
} else if ( SELECT_CASE_var == LoadBased ) {
} else if ( SELECT_CASE_var == LoadBased || SELECT_CASE_var == CCM_ASHRAE ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Single zone VAV is actually the same as Load based, with new logic for control.

@rraustad
Copy link
Contributor Author

The 1 test had: MTR big diffs still bugs me. So this time I added report variables that make up the Electricity:HVAC meter hoping to track this down. Comparing current develop to this branch still gives no diff's. I'm confused as to why CI shows diff's and I don't, but at least I tried.

HVACTemplate-5ZoneUnitarySystem.csv vs HVACTemplate-5ZoneUnitarySystem_new.csv
All Records are Equal.    48 records compared
* * * * * * * * * * * * 
HVACTemplate-5ZoneUnitarySystemMeter.csv vs HVACTemplate-5ZoneUnitarySystem_newMeter.csv
All Records are Equal.    2 records compared
* * * * * * * * * * * * 
HVACTemplate-5ZoneUnitarySystemZsz.csv vs HVACTemplate-5ZoneUnitarySystem_newZsz.csv
All Records are Equal.    99 records compared
* * * * * * * * * * * * 
HVACTemplate-5ZoneUnitarySystemSsz.csv vs HVACTemplate-5ZoneUnitarySystem_newSsz.csv
All Records are Equal.    98 records compared

For Meter=Electricity:HVAC [J], ResourceType=Electricity, Group=HVAC, contents are:
  SYS 2 FURNACE DX COOL MULTISPD SUPPLY FAN:Fan Electric Energy
  SYS 1 FURNACE DX COOL SNGLSPD SUPPLY FAN:Fan Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD SUPPLY FAN:Fan Electric Energy
  SYS 4 HEAT PUMP WATERSOURCE SNGLSPD SUPPLY FAN:Fan Electric Energy
  SYS 5 HW CHW SYSTEM SUPPLY FAN:Fan Electric Energy
  SYS 2 FURNACE DX COOL MULTISPD HEATING COIL:Heating Coil Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD SUPP HEAT OR REHEAT COIL:Heating Coil Electric Energy
  SYS 4 HEAT PUMP WATERSOURCE SNGLSPD SUPP HEAT OR REHEAT COIL:Heating Coil Electric Energy
  SYS 1 FURNACE DX COOL SNGLSPD HEATING COIL:Heating Coil Electric Energy
  SYS 1 FURNACE DX COOL SNGLSPD COOLING COIL:Cooling Coil Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD COOLING COIL:Cooling Coil Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD HEATING COIL:Heating Coil Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD HEATING COIL:Heating Coil Defrost Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD HEATING COIL:Heating Coil Crankcase Heater Electric Energy
  SYS 2 FURNACE DX COOL MULTISPD COOLING COIL:Cooling Coil Electric Energy
  SYS 4 HEAT PUMP WATERSOURCE SNGLSPD COOLING COIL:Cooling Coil Electric Energy
  SYS 4 HEAT PUMP WATERSOURCE SNGLSPD HEATING COIL:Heating Coil Electric Energy
  SYS 1 FURNACE DX COOL SNGLSPD UNITARY SYSTEM:Unitary System Heating Ancillary Electric Energy
  SYS 2 FURNACE DX COOL MULTISPD UNITARY SYSTEM:Unitary System Heating Ancillary Electric Energy
  SYS 2 FURNACE DX COOL MULTISPD UNITARY SYSTEM:Unitary System Cooling Ancillary Electric Energy
  SYS 3 HEAT PUMP AIRSOURCE SNGLSPD UNITARY SYSTEM:Unitary System Heating Ancillary Electric Energy
  SYS 4 HEAT PUMP WATERSOURCE SNGLSPD UNITARY SYSTEM:Unitary System Heating Ancillary Electric Energy
  SYS 5 HW CHW SYSTEM UNITARY SYSTEM:Unitary System Heating Ancillary Electric Energy

@Myoldmopar
Copy link
Member

Our branches are moving much faster than poor CI can keep up with, so I wouldn't be completely surprised if the diffs are unrelated. As far as you're concerned, are you done with this branch?

@rraustad
Copy link
Contributor Author

Yes, this should be complete. I asked Safvat today about the status of running CI on FSEC blades and he said there has been no movement. You should consider that since his blades would be smokin' right now.

@EnergyArchmage
Copy link
Contributor

I have looked at this code briefly and agree it won't conflict with the VS coil project very much. Probably makes sense to merge this one before #5943, then I will fix up whatever conflicts there are.

@Myoldmopar
Copy link
Member

OK, I'm willing to risk just a little to get this in before CI is all done. I'll merge it now. @EnergyArchmage prepare for resolving any merge issues.

@Myoldmopar Myoldmopar merged commit 915298a into develop Feb 16, 2017
@Myoldmopar Myoldmopar deleted the Branch_PT133379791-New-Feature---SZVAV-Code-and-Unit-Tests branch February 16, 2017 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants