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.14k stars 389 forks source link

"Other" heating subcategory appears (incorrectly) #7556

Closed shorowit closed 4 years ago

shorowit commented 5 years ago

Issue overview

See IDF below where >12 GJ of energy appears in an "Other" heating subcategory in the End Uses By Subcategory output table. The subcategory is being automatically created. Across several hundred test IDFs, this is the only file where this subcategory appears. In every other test file, heating energy correctly shows up in the "General" subcategory. (This particular test file includes a complex set of HVAC systems serving a single conditioned zone.)

in.idf.txt

Details

Some additional details for this issue (if relevant):

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

JasonGlazer commented 4 years ago

@shorowit I'm having trouble tracking down why this file is behaving differently. Would it be possible for you to provide a file that is as close as possible to this one but worked?

shorowit commented 4 years ago

@JasonGlazer Try these two files.

The good file has a AirLoopHVAC:UnitarySystem serving 100% of the conditioned zone's cooling load and 30% of the heating load, with a ZoneHVAC:Baseboard:Convective:Electric meeting the other 70% of the heating load.

The bad file has a AirLoopHVAC:UnitarySystem serving 100% of the conditioned zone's cooling load and 30% of the heating load, with a ZoneHVAC:Baseboard:Convective:Water meeting the other 70% of the heating load.

They are otherwise essentially the same.

bad.idf.txt good.idf.txt

JasonGlazer commented 4 years ago

@shorowit The more I look at the code, the more I think this is exactly as it is intended to work. The Boiler:HotWater using electricity in the "bad" file has "General" as a end-use subcategory which is a little confusing because that is the default name for the first row for each end-use in the "End Uses By Subcategory" table. If that is removed, it shows as "Boiler." The rest of the heating electricity use is from the DX heating coils which makes sense but they are not really categorized as "general" but that is just the default name for that row if no end-use subcategory names are used. Because they are not categorized, they are being lumped into "other." The algorithm for that table makes a row for each named end use subcategory and then adds "other" if the total for the end-use is greater than the sum of the named end-use subcategories. I would guess in your other files you don't have any named end use subcategories for heating.

I hope this is clear. Let me know if you have questions. If you don't I will probably just close this issue.

shorowit commented 4 years ago

@JasonGlazer Thanks for looking into this. I'm not really following. Maybe you can help me understand by answering this question: In the "bad" file, what changes would I need to make to the IDF so that the "other" end use subcategory disappears -- and behaves like the "good" file?

(For what it's worth, apparently OpenStudio is automatically setting the Boiler:HotWater end use subcategory to "General".)

shorowit commented 4 years ago

@JasonGlazer Just to provide some more information, here are a handful of similar test files we're running. (We have dozens more with different HVAC configurations.)

IDF Heating System IDF Cooling System Has "other"? Notes
ZoneHVAC:Baseboard:Convective:Water None No
ZoneHVAC:Baseboard:Convective:Water AirLoopHVAC:UnitarySystem No
None AirLoopHVAC:UnitarySystem No
ZoneHVAC:Baseboard:Convective:Electric & AirLoopHVAC:UnitarySystem AirLoopHVAC:UnitarySystem No good.idf.txt
ZoneHVAC:Baseboard:Convective:Water & AirLoopHVAC:UnitarySystem AirLoopHVAC:UnitarySystem Yes bad.idf.txt

Because so many HVAC test files don't produce an "other" subcategory end use, it leads me to believe that something isn't quite right.

JasonGlazer commented 4 years ago

I would say that nothing is really broken or wrong, it is just the combination of electric heating items that is causing a different arrangement of results in that table. I would guess that the boiler may be the only object that does electric heating in your suite that has an End-Use Subcategory field. The other electric heating items probably don't have that field.

Could it be better? Certainly! It would be nice if objects that don't have an End-Use Subcategory field are treated like they are just considered a "General" end use subcategory. I am a little worried that such a change might have repercussions for the arrangement of that table for lots of people.

I would appreciate some input from @mjwitte and @Myoldmopar on this. Should we treat objects that don't have an End-Use Subcategory field like they a "General" end use subcategory?

mjwitte commented 4 years ago

Based on a brief review of this, I think that SetUpOutputVariable should apply General as the default Subcategory for any metered variable with a main end-use specified. I'm kind of surprised that's not how it is already. "Other" should never really happen - we probably debated this when the Other row was added originally.

In addition we could add some hard-coded subcategories as suggested in #4951. That's apparently already been done for the Boiler parasitics (there's no input field for this subcategory):

 Meters for 1254,BOILER:Boiler Ancillary Electric Energy [J]
  OnMeter=Electricity:Facility [J]
  OnMeter=Electricity:Plant [J]
  OnMeter=Heating:Electricity [J]
  OnMeter=Boiler Parasitic:Heating:Electricity [J]
shorowit commented 4 years ago

Based on a brief review of this, I think that SetUpOutputVariable should apply General as the default Subcategory for any metered variable with a main end-use specified. I'm kind of surprised that's not how it is already. "Other" should never really happen - we probably debated this when the Other row was added originally.

👍 I'm supportive of this.

rraustad commented 4 years ago

More basic is the overrides that are not consistent. Why isn't indexGroupKey the 8th argument in each of these functions. Couldn't these be reduced to a single function?

void SetupOutputVariable(std::string const &VariableName,           // String Name of variable (with units)
                         OutputProcessor::Unit const &VariableUnit, // Actual units corresponding to the actual variable
                         Real64 &ActualVariable,                    // Actual Variable, used to set up pointer
                         std::string const &IndexTypeKey,           // Zone, HeatBalance=1, HVAC, System, Plant=2
                         std::string const &VariableTypeKey,        // State, Average=1, NonState, Sum=2
                         std::string const &KeyedValue,             // Associated Key for this variable
                         Optional_string_const ReportFreq = _,      // Internal use -- causes reporting at this freqency
                         Optional_string_const ResourceTypeKey = _, // Meter Resource Type (Electricity, Gas, etc)
                         Optional_string_const EndUseKey = _,       // Meter End Use Key (Lights, Heating, Cooling, etc)
                         Optional_string_const EndUseSubKey = _,    // Meter End Use Sub Key (General Lights, Task Lights, etc)
                         Optional_string_const GroupKey = _,        // Meter Super Group Key (Building, System, Plant)
                         Optional_string_const ZoneKey = _,         // Meter Zone Key (zone name)
                         Optional_int_const ZoneMult = _,           // Zone Multiplier, defaults to 1
                         Optional_int_const ZoneListMult = _,       // Zone List Multiplier, defaults to 1
                         Optional_int_const indexGroupKey = _,      // Group identifier for SQL output
                         Optional_string_const customUnitName = _   // the custom name for the units from EMS definition of units
);

void SetupOutputVariable(std::string const &VariableName,           // String Name of variable
                         OutputProcessor::Unit const &VariableUnit, // Actual units corresponding to the actual variable
                         int &ActualVariable,                       // Actual Variable, used to set up pointer
                         std::string const &IndexTypeKey,           // Zone, HeatBalance=1, HVAC, System, Plant=2
                         std::string const &VariableTypeKey,        // State, Average=1, NonState, Sum=2
                         std::string const &KeyedValue,             // Associated Key for this variable
                         Optional_string_const ReportFreq = _,      // Internal use -- causes reporting at this freqency
                         Optional_int_const indexGroupKey = _       // Group identifier for SQL output
);

void SetupOutputVariable(std::string const &VariableName,           // String Name of variable
                         OutputProcessor::Unit const &VariableUnit, // Actual units corresponding to the actual variable
                         Real64 &ActualVariable,                    // Actual Variable, used to set up pointer
                         std::string const &IndexTypeKey,           // Zone, HeatBalance=1, HVAC, System, Plant=2
                         std::string const &VariableTypeKey,        // State, Average=1, NonState, Sum=2
                         int const KeyedValue,                      // Associated Key for this variable
                         Optional_string_const ReportFreq = _,      // Internal use -- causes reporting at this freqency
                         Optional_string_const ResourceTypeKey = _, // Meter Resource Type (Electricity, Gas, etc)
                         Optional_string_const EndUseKey = _,       // Meter End Use Key (Lights, Heating, Cooling, etc)
                         Optional_string_const EndUseSubKey = _,    // Meter End Use Sub Key (General Lights, Task Lights, etc)
                         Optional_string_const GroupKey = _,        // Meter Super Group Key (Building, System, Plant)
                         Optional_string_const ZoneKey = _,         // Meter Zone Key (zone name)
                         Optional_int_const ZoneMult = _,           // Zone Multiplier, defaults to 1
                         Optional_int_const ZoneListMult = _,       // Zone List Multiplier, defaults to 1
                         Optional_int_const indexGroupKey = _       // Group identifier for SQL output
);
mjwitte commented 4 years ago

@rraustad Agreed at some point we need a sweep to clean these up and get of the optional argument stuff. And we should probably have two distinct function names, something like SetupOutputVariable and SetupMeteredVariable.

JasonGlazer commented 4 years ago

@rraustad Yes, this does need cleaning up.

JasonGlazer commented 4 years ago

In general, I think the concept of end-use subcategory is good and inputs should be added to objects so that users can define them for all objects that consume electricity or a fuel.

mjwitte commented 4 years ago

OK, but that could involve many objects. Is there any objection to a first step of defaulting to General?

JasonGlazer commented 4 years ago

I think it is a good idea. I'm actually surprised it wasn't that way originally. My only concern is that it might change a lot of tabular output files both in the test suite and for users in general but I think we should go ahead with the change (which is probably just a single line of code).

JasonGlazer commented 4 years ago

I just added a new feature request #7673 to not lose track that the end use subcategory field should be added to additional objects.

shorowit commented 4 years ago

Thanks @JasonGlazer!