Coil:Heating:Fuel changed to accept Propane

[nmerket]

For the Coil:Heating:Fuel object which recently had the Fuel Type input added to it. the enumeration in the idd wasn't the same as what the input function was expecting. This corrects the idd to list "Propane" (the correct choice) instead of "PropaneGas".

Note that there is a mix of Propane vs PropaneGas in other objects, but Propane is preferred. Other uses of PropaneGas may be addressed in #5941.

@macumber @kbenne @Myoldmopar @shorowit

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

[nmerket]

Text editor problems. I'll get these indents fixed.

[mjwitte]

@nmerket Well, that's annoying. Searching the idd, it seems we have a mix of "Propane" and "PropaneGas". Here are instances of "Propane" in the IDD:

EnergyManagementSystem:MeteredOutputVariable,
  A5 , \field Resource Type
       \required-field
       \note choose the type of fuel, water, electricity, pollution or heat rate that should be metered.
       \type choice
       \key Propane

LifeCycleCost:UsePriceEscalation
   A2,  \field Resource
        \required-field
        \type choice
        \key Propane

LifeCycleCost:UseAdjustment
<similar>

FuelFactors
  A1,   \field Existing Fuel Resource Name
        \type choice
        \key Electricity
        \key NaturalGas
        \key FuelOil#1
        \key FuelOil#2
        \key Coal
        \key Gasoline
        \key Propane

Propane also appears in Generator:FuelSupply but that's in a list with things like Methane and Hexane, so that's ok.

So, it may be better at this point to leave the code alone and simply change the IDD and docs to "Propane". At some point, we should clean the code to standardize all fuel type inputs to use the same list (and the same function to check the list) and get rid of all the synonyms, but that will require transition rules so that's beyond scope of this fix.

[nmerket]

That's fine with me. I knew it could go either way: change the idd or change the code. I chose this way because the open studio guys get mad at me when I change the names of things. However this is new enough that I doubt it will be causing too much problems for people. Nevertheless I will let those guys chime in before before changing the idd again.

[mjwitte]

@nmerket On second thought, keeping PropaneGas here and changing the few uses of "Propane" in the IDD is less disruptive. I'd say stay the course here and new issue #5941 will do the other cleanups.

Striking this per comments below.

[macumber]

My preference would be to take two cycles to make this change. In the first cycle you can add synonyms to the IDD (e.g. support Propane and PropaneGas) but mark one as deprecated (might even issue warnings in the E+ output). Then in the second cycle you can remove support for deprecated terms.

[macumber]

The real issue I guess is which term shows up in the outputs, I would not change this in a bug fix patch but wait to change it in a major version.

[mjwitte]

@macumber Meter names use "Gas" (for natural gas) and "Propane". That won't change here, but could change as part of #5941.

[mjwitte]

And table output headings are currently "Natural Gas" and "Additional Fuel" (which I presume includes everything except Electricity, NaturalGas, and district heating cooling).

[nmerket]

So, for this if I add "Propane" will that be workable?

[JasonGlazer]

In the End Uses table of the Annual Building Utility Performance Summary report, the "Additional Fuel" column includes: Gasoline, Diesel, Coal, FuelOil#1, FuelOil#2, Propane, OtherFuel1, OtherFuel2.

Lets make sure the meter names and inputs are consistent.

[nmerket]

@Myoldmopar what's our final decision here? How do I proceed?

[nrel-bot-3]

@nmerket @lgentile it has been 14 days since this pull request was last updated.

[nmerket]

@Myoldmopar this is ready to merge into develop. Let's not worry about a special bugfix release. We'll catch it the next time around.

[Myoldmopar]

OK, seems like consensus on using Propane. Which makes the most sense to me. If there are other inconsistencies to deal with, we can make a new issue. But this, specifically just fixing the issue introduced in 8.6, seems like it can drop in without more overhead. Speak your peace as I'll merge shortly.

[mjwitte]

@Myoldmopar But the changes currently in this PR use PropaneGas, not Propane.

[Myoldmopar]

😄 Thanks @mjwitte . OK, so the actual work here is to transition away PropaneGas??

[nmerket]

The point of this was to make the idd that shipped with 8.6 work as advertised. I'm fine with having it be Propane instead. Let me know what you want me to do.

[Myoldmopar]

Propane it is. So then we just need a simple transition rule?

[mjwitte]

I don't think we need a transition rule. The current code only accepts Propane. The only thing that was wrong is the IDD choice is PropaneGas.

[nmerket]

No, @mjwitte, the idd only accepts PropaneGas right now, but the code expects Propane. Currently this bug fix changes the code to match the idd. I can change the idd so that it expects Propane instead, but I think we may need a transition because the idd changed. Maybe it doesn't matter because it didn't work anyway, so there's not going to be any models out there with it specified as PropaneGas.

[mjwitte]

That was my point - any existing file with PropaneGas fatals out anyway, so transition isn't really needed. But I suppose it doesn't hurt to have it there just in case.

[Myoldmopar]

Yeah, if someone has an 8.5 input file, and transitions it up to 8.7, having a transition rule in place to put it back to just Propane would make it seamless.

[Myoldmopar]

Nevermind...I'm still groggy apparently.

[nmerket]

@mjwitte @Myoldmopar Changed the idd to accept only "Propane". No transition. You're welcome to add one if it's important to you. Otherwise this is ready to merge.

[mjwitte]

@nmerket Thanks. I just checked the I/O Ref, it needs to be changed also. Sorry I didn't think to check that before.

[nmerket]

Good catch. I'll fix that too.

[nmerket]

@mjwitte I updated the docs.

[mjwitte]

@nmerket @Myoldmopar Just waiting for the last CI reports, then I will merge.

Any reason we shouldn't also change Coil:Heating:Gas:MultiStage to Coil:Heating:Fuel:MultiStage, and Humidifier:Steam:Gas?

[nmerket]

No reason not to. It just wasn't an immediate need.