Defrost Model Improvements #1674

yzhou601 · 2024-04-04T08:16:58Z

Pull Request Description

Allow an optional improved defrost model

Breaking change: Replaces SimulationControl/TemperatureCapacitanceMultiplier with SimulationControl/AdvancedResearchFeatures/TemperatureCapacitanceMultiplier.

Checklist

PR Author: Check these when they're done. Not all may apply. ~~strikethrough~~ and check any that do not apply.

PR Reviewer: Verify each has been completed.

Schematron validator (EPvalidator.xml) has been updated
Sample files have been added/updated (openstudio tasks.rb update_hpxmls)
Tests have been added/updated (e.g., HPXMLtoOpenStudio/tests/test*.rb and/or workflow/tests/test*.rb)
Documentation has been updated
Changelog has been updated
openstudio tasks.rb update_measures has been run
No unexpected changes to simulation results of sample files

…o defrost_improvements

HPXMLtoOpenStudio/tests/test_hvac.rb

HPXMLtoOpenStudio/resources/hvac.rb

…o defrost_improvements

yzhou601 · 2024-05-01T22:40:26Z

I think this is ready for review @jonwinkler @shorowit

HPXMLtoOpenStudio/resources/hvac.rb

…o defrost_improvements # Conflicts: # BuildResidentialHPXML/measure.rb # BuildResidentialHPXML/measure.xml # HPXMLtoOpenStudio/measure.xml # ReportSimulationOutput/measure.rb # ReportSimulationOutput/measure.xml # workflow/tests/base_results/results_simulations_bills.csv # workflow/tests/base_results/results_simulations_misc.csv

shorowit

I was trying to find a description of what is "advanced" about this defrost approach and how it differs from our standard defrost model. It's not mentioned anywhere -- not in the documentation, not in the PR description, not in the changelog, not in the BuildResHPXML measure argument description. How is someone supposed to choose whether to use this?

Also, should it be described as a "research" feature, like we're doing for some GEB features?

Finally, I can't help but feel like there should be a single top-level input rather than a per-system input. Surely you would want to use this for any HVAC system that has defrost controls, no?

shorowit · 2024-05-08T18:22:25Z

I keep wondering if we should group a bunch of "advanced research" inputs together in the HPXML header:

This
GEB HVAC controls
Water heater tank model?
Temperature capacitance multiplier?
???

yzhou601 · 2024-05-08T18:34:07Z

I keep wondering if we should group a bunch of "advanced research" inputs together in the HPXML header:

This

GEB HVAC controls

Water heater tank model?

Temperature capacitance multiplier?

???

Ha I was just about to ask whether there should be a top level element like that. I like this idea.

yzhou601 · 2024-05-08T18:37:25Z

I was trying to find a description of what is "advanced" about this defrost approach and how it differs from our standard defrost model. It's not mentioned anywhere -- not in the documentation, not in the PR description, not in the changelog, not in the BuildResHPXML measure argument description. How is someone supposed to choose whether to use this?

Also, should it be described as a "research" feature, like we're doing for some GEB features?

Finally, I can't help but feel like there should be a single top-level input rather than a per-system input. Surely you would want to use this for any HVAC system that has defrost controls, no?

Good point Scott, I agree that the documentation is not perfect, I'll add more detailed descriptions to these files.

…o defrost_improvements

yzhou601 · 2024-05-08T21:26:10Z

I was trying to find a description of what is "advanced" about this defrost approach and how it differs from our standard defrost model. It's not mentioned anywhere -- not in the documentation, not in the PR description, not in the changelog, not in the BuildResHPXML measure argument description. How is someone supposed to choose whether to use this?

Also, should it be described as a "research" feature, like we're doing for some GEB features?

Finally, I can't help but feel like there should be a single top-level input rather than a per-system input. Surely you would want to use this for any HVAC system that has defrost controls, no?

@jonwinkler I guess this is a question for you, is it possible to have use cases that we only want to apply this defrost approach for some systems but keep the previous approach for others under the same HPXML Building?

jonwinkler · 2024-05-08T21:39:53Z

I was trying to find a description of what is "advanced" about this defrost approach and how it differs from our standard defrost model. It's not mentioned anywhere -- not in the documentation, not in the PR description, not in the changelog, not in the BuildResHPXML measure argument description. How is someone supposed to choose whether to use this?
Also, should it be described as a "research" feature, like we're doing for some GEB features?
Finally, I can't help but feel like there should be a single top-level input rather than a per-system input. Surely you would want to use this for any HVAC system that has defrost controls, no?

@jonwinkler I guess this is a question for you, is it possible to have use cases that we only want to apply this defrost approach for some systems but keep the previous approach for others under the same HPXML Building?

@yzhou601 I can't think of any. Do you mean one building with multiple heat pumps where we want to use the default approach for one heat pump and the advanced approach for the other? I don't think that would be the case.

yzhou601 · 2024-05-08T21:41:00Z

I was trying to find a description of what is "advanced" about this defrost approach and how it differs from our standard defrost model. It's not mentioned anywhere -- not in the documentation, not in the PR description, not in the changelog, not in the BuildResHPXML measure argument description. How is someone supposed to choose whether to use this?
Also, should it be described as a "research" feature, like we're doing for some GEB features?
Finally, I can't help but feel like there should be a single top-level input rather than a per-system input. Surely you would want to use this for any HVAC system that has defrost controls, no?

@jonwinkler I guess this is a question for you, is it possible to have use cases that we only want to apply this defrost approach for some systems but keep the previous approach for others under the same HPXML Building?

@yzhou601 I can't think of any. Do you mean one building with multiple heat pumps where we want to use the default approach for one heat pump and the advanced approach for the other? I don't think that would be the case.

Yes that's what I mean, if we move the input to the top level then we can only specify one for the whole building, so that's why I'm asking. Great, then I'm going to proceed in this way.

shorowit · 2024-05-08T21:44:48Z

I keep wondering if we should group a bunch of "advanced research" inputs together in the HPXML header:

This

GEB HVAC controls

Water heater tank model?

Temperature capacitance multiplier?

???

Ha I was just about to ask whether there should be a top level element like that. I like this idea.

How about under SimulationControl? Right now, TemperatureCapacitanceMultiplier is directly under it. But we could change it to be SimulationControl/AdvancedResearchFeatures and put TemperatureCapacitanceMultiplier, AdvancedDefrostModel, etc all under it.

(Not sure if the name should be "AdvancedFeatures", "ResearchFeatures", or "AdvancedResearchFeatures" 😄)

shorowit · 2024-05-08T21:46:10Z

And for defrost, instead of a boolean, maybe it should be choice element. DefrostModelType with "standard" and "advanced" or whatever the right descriptions are.

yzhou601 · 2024-05-08T22:05:06Z

I keep wondering if we should group a bunch of "advanced research" inputs together in the HPXML header:

This

GEB HVAC controls

Water heater tank model?

Temperature capacitance multiplier?

???

Ha I was just about to ask whether there should be a top level element like that. I like this idea.

How about under SimulationControl? Right now, TemperatureCapacitanceMultiplier is directly under it. But we could change it to be SimulationControl/AdvancedResearchFeatures and put TemperatureCapacitanceMultiplier, AdvancedDefrostModel, etc all under it.

(Not sure if the name should be "AdvancedFeatures", "ResearchFeatures", or "AdvancedResearchFeatures" 😄)

For geb features, my previous changes are made to be under /HPXML/Building/BuildingDetails/BuildingSummary/extension/ so that those are saved at building level. SimulationControl is at HPXML level, so is it possible that for some use case, within one HPXML element, Building elements use different inputs for these features?

shorowit · 2024-05-08T22:24:32Z

I can't imagine someone would want to use different assumptions for different Building elements. I suggest we keep it simple.

…o defrost_improvements

…udio-HPXML into defrost_improvements

…o defrost_improvements # Conflicts: # HPXMLtoOpenStudio/measure.xml # workflow/tests/base_results/results_simulations_bills.csv # workflow/tests/base_results/results_simulations_energy.csv # workflow/tests/base_results/results_simulations_hvac.csv # workflow/tests/base_results/results_simulations_loads.csv # workflow/tests/base_results/results_simulations_misc.csv

shorowit

The implementation overall looks very reasonable, though I didn't scrutinize the calculations or EnergyPlus results at all. I assume that you and Jon have been testing this and are happy with the results.

I did push a commit with a few language changes, you might look that over. Otherwise, just a few things I noticed below.

HPXMLtoOpenStudio/measure.rb

HPXMLtoOpenStudio/resources/hpxml_defaults.rb

HPXMLtoOpenStudio/resources/hvac.rb

yzhou601 and others added 3 commits March 25, 2024 10:31

change E+ defrost inputs

0b08c5f

Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML int…

dc68b98

…o defrost_improvements

first cut of defrost EMS and other equipment objects

152071e

yzhou601 added the enhancement New feature or request label Apr 4, 2024

yzhou601 self-assigned this Apr 4, 2024

yzhou601 added 6 commits April 4, 2024 09:33

bugfix, default, and more test files

1596977

docs and hvac unit test added.

6330e3c

Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML int…

94b7513

…o defrost_improvements

unit multiplier bugfix, update q_dot p_dot calculation

b35a7b6

p_dot_blower and p_dot_odu_fan

ef48fd2

bugfix

af7ab86

yzhou601 commented Apr 25, 2024

View reviewed changes

HPXMLtoOpenStudio/tests/test_hvac.rb Outdated Show resolved Hide resolved

yzhou601 commented Apr 25, 2024

View reviewed changes

HPXMLtoOpenStudio/resources/hvac.rb Outdated Show resolved Hide resolved

yzhou601 and others added 4 commits April 25, 2024 14:48

unit multiplier fix with blower and fan power accounted

f186290

Merge branch 'master' of https://github.com/NREL/OpenStudio-HPXML int…

502f687

…o defrost_improvements

updates for ductless systems

bb25da0

Latest results.

a4c855c

yzhou601 marked this pull request as ready for review May 1, 2024 22:40

yzhou601 requested review from jonwinkler and shorowit May 1, 2024 22:40