-
Notifications
You must be signed in to change notification settings - Fork 57
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
Heating system -> heat pump backup #1074
Conversation
measures/ApplyUpgrade/measure.rb
Outdated
heating_system = get_heating_system(hpxml) | ||
|
||
if not heating_system.nil? | ||
if [HPXML::HVACTypeFurnace, HPXML::HVACTypeFixedHeater].include?(heating_system.heating_system_type) # Integrated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels pretty brittle/risky. It'll be easy for this code to not get updated if new heating properties or system types are added.
I haven't thought about it as hard as you have, but I don't really understand the reason for choosing this approach. Why not keep things how they were but simply preserve the heating capacity when "separate" is chosen and there is an existing system?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Furnace, for example, is not a valid "second heating system".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it more, I realize part of the motivation is to allow preserving not just capacity but also efficiency/fueltype. So you'd be able to specify a single heat pump upgrade that applies to many different existing fossil fuel systems, rather than a separate heat pump upgrade for each existing system type. I just worry a bit about all the code that is being introduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Furnace, for example, is not a valid "second heating system".
If the furnace is backup to a central ASHP, then it is allowed (as an integrated backup system).
If the furnace is backup to a ductless mini-split, then I agree it's not currently allowed. I would suggest we update BuildResHPXML to allow it. So long as we only end up with one duct system for the home, I think it should be straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "all the code" you are referring to the "integrated" (i.e., furnace, fixed heater) vs "separate" switch, and having all these BuildResHPXML arguments called out explicitly? What would you suggest as an alternative? (Keep in mind that we do already call out quite a few BuildResHPXML and BuildResSchFile arguments in these meta measures.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Furnace, for example, is not a valid "second heating system".
If the furnace is backup to a central ASHP, then it is allowed (as an integrated backup system).
If the furnace is backup to a ductless mini-split, then I agree it's not currently allowed. I would suggest we update BuildResHPXML to allow it. So long as we only end up with one duct system for the home, I think it should be straightforward.
By "second heating system", I meant it's not allowed here: https://github.com/NREL/OpenStudio-HPXML/blob/master/BuildResidentialHPXML/measure.rb#L1340-L1354.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "second heating system", I meant it's not allowed here: https://github.com/NREL/OpenStudio-HPXML/blob/master/BuildResidentialHPXML/measure.rb#L1340-L1354.
Yeah, I understand. Okay, what I said earlier wasn't quite right, let me try that again.
We should update the BuildResHPXML measure to allow a furnace as a second heating system -- so long as the home doesn't end up with two duct systems.
- If there is a ducted heat pump w/ backup furnace, the backup type needs to be "integrated". (If it was "separate", that would imply two duct systems. OS-HPXML can handle that, but that's too much complexity for the BuildResHPXML measure and would be a very uncommon situation.)
- If there is a ductless heat pump w/ backup furnace, the backup type needs to be "separate". (A ductless heat pump can't have ducted "integrated" backup, that wouldn't make any sense. I wonder what OS-HPXML does in that situation...)
@whiphi92 You are talking about HP to HP upgrades here? |
@whiphi92 I think this gets complicated when considering upgrades from a primary heating system that serves less than 100% of the heating load (i.e., there is a secondary system in the existing building). This is prompted by the recently merged #1093. I'm not sure if this is related to the issue you bring up above. I think the main question is: should the fraction of the heating load served by the upgraded heat pump equal the fraction served by the existing system (i.e., we retain the secondary system), or should it be set to 100% regardless (i.e., we ignore the secondary system)? (Perhaps this only applies when the heat pump backup type is integrated; when separate, the backup system fraction is not allowed, and we're forced to ignore the secondary system.) |
@joseph-robertson my intuition is that we would want the heat pump to serve 100% of the load of the dwelling and its backup be the exact size that the original primary system was (even if it only served a fractional percentage of the full load of the dwelling). Does this make sense? Ideally, I suppose, would be that the user can determine whether the heat pump upgrade will serve 100% of the load or just stay the same. We do this with AC via the partial conditioning argument. |
This is the scenario I was trying to convey above: Existing
Upgraded
Should (A) x=60 and y=40, or (B) x=100 and y=0 (i.e., the Fireplace goes away)? Sounds like it should be (B). Is that right? |
There's no single answer, it will depend on the situation. Why not default to A, since the user can choose B if they set secondary heating to None as part of the upgrade (right?). |
Good point. I'll try rolling with that. |
@whiphi92 This is ready for another review. |
Plan is to create a new RTD page "Heat Pump Upgrades" (or similar) that addresses the following:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, this looks great.
We need to work on documentation about default values for compressor and backup lockouts and other heat pump related modeling.
Work on that documentation is being performed here.
Pull Request Description
Closes #942.
Uses ResStockArguments to add a new boolean argument
heat_pump_backup_use_existing_system
. When set totrue
in the lookup (i.e., as an upgrade option) the heating system becomes the heat pump backup:*If existing heating system is an electric furnace, it likely wouldn't be used as integrated backup.
Fuel type, efficiency, and capacity are all retained.
If the existing heating system is a heat pump or shared system, it does not get set as the heat pump backup.
This PR also updates
national_upgrades.yml
andtesting_upgrades.yml
to demonstrate the newHeat Pump Backup|Use Existing System
option for the "ASHP" and "MSHP" upgrades. Also, adds a new "Ductless MSHP" upgrade to these yml files.Checklist
Not all may apply:
Documentation has been updatedopenstudio tasks.rb update_measures
has been run