search

NREL / buildstockbatch

Other
21 stars 14 forks source link

ResStock-HPXML: optional utility bill reporting arguments #451

Closed joseph-robertson closed 2 months ago

joseph-robertson commented 4 months ago

Pull Request Description

User now has the ability to optionally set utility bill reporting arguments:

These will live under the simulation_output_report section of the yml file.

Argument include_annual_bills=True will generate results_bills.csv as well as register annual bills with the OpenStudio runner. Argument include_monthly_bills will generate results_bills_monthly.csv as well as register monthly bills with the OpenStudio runner.

(Related to https://github.com/NREL/resstock/pull/1246 and https://github.com/NREL/buildstockbatch/pull/383.)

Checklist

Not all may apply

shorowit commented 4 months ago

Does it really make sense to expose the new YAML inputs to a user? Why not just set both include_foo and register_foo to the same value based on the existing YAML input? For ResStock, doesn't seem like there's any harm to registering values unnecessarily...

joseph-robertson commented 4 months ago

Does it really make sense to expose the new YAML inputs to a user? Why not just set both include_foo and register_foo to the same value based on the existing YAML input? For ResStock, doesn't seem like there's any harm to registering values unnecessarily...

What's the existing YAML input? Currently these are hardset: include_annual_bills=True and include_monthly_bills=False.

If we hardset register_monthly_bills=True, couldn't your results.csv be crazy huge depending on how many bill scenarios are defined?

shorowit commented 4 months ago

Oh sorry, I didn't realize there wasn't an input before. If you are adding inputs, I guess I'm advocating for two inputs instead of four. Only the include_foo arguments affect the results CSVs, right? The register_foo arguments only affect the results.json.

At the end of the day, the register_foo arguments are what ResStock needs to output utility bill results, right? I foresee a situation where a user sets include_foo=true and register_foo=false, runs millions of simulations, gets no utility bill results, and is not happy.

joseph-robertson commented 4 months ago

Hmm I do see your point. But on the flip side, seems like it'd be nice to save space by avoiding writing results_bills.csv and results_monthly_bills.csv.

github-actions[bot] commented 4 months ago

File Coverage
All files 87% :white_check_mark:
base.py 91% :white_check_mark:
exc.py 57% :white_check_mark:
hpc.py 78% :white_check_mark:
local.py 70% :white_check_mark:
postprocessing.py 84% :white_check_mark:
utils.py 92% :white_check_mark:
cloud/docker_base.py 88% :white_check_mark:
sampler/base.py 79% :white_check_mark:
sampler/downselect.py 33% :white_check_mark:
sampler/precomputed.py 93% :white_check_mark:
sampler/residential_quota.py 61% :white_check_mark:
test/shared_testing_stuff.py 85% :white_check_mark:
test/test_docker.py 33% :white_check_mark:
test/test_local.py 97% :white_check_mark:
test/test_validation.py 97% :white_check_mark:
workflow_generator/base.py 90% :white_check_mark:
workflow_generator/commercial.py 53% :white_check_mark:
workflow_generator/residential_hpxml.py 82% :white_check_mark:

Minimum allowed coverage is 33%

Generated by :monkey: cobertura-action against cbb4e726060fab3473f0e344990d7f68f42a4d83

afontani commented 4 months ago

I want to point out that the actual way utility bills are reported is still mostly billing months. A bill dated March 1st is for February consumption. With the adoption of AMI metering, the bills are becoming much closer to calendar months. But utilities are still mixed in how they report bills. Some use the AMI timeseries aggregates for the month, others use the billing statements. Just a comment.

joseph-robertson commented 4 months ago

I tested that this branch works here in resstock.