Skip to content
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

Add water heater location #1125

Merged
merged 35 commits into from
Sep 25, 2023
Merged

Add water heater location #1125

merged 35 commits into from
Sep 25, 2023

Conversation

lixiliu
Copy link
Contributor

@lixiliu lixiliu commented Aug 24, 2023

Pull Request Description

Addresses Issue: #1094
ResStock-estimation companion PR: https://github.com/NREL/resstock-estimation/pull/385

  • Added Water Heater Location (include fixing other tsvs to remove non-zero Void options)
  • Previously, water heater location is assigned based on OS-HPXML default logics:
    first available space type:
    - IECC zones 1-3, excluding 3A: “garage”, “living space”
    - IECC zones 3A, 4-8, unknown: “basement - conditioned”, “basement - unconditioned”, “living space”
  • Now, Water Heater Location is informed by RECS 2020 with restrictions on outside and vented space installations for IECC zones 4-8 due to pipe-freezing risk, as advised by @jmaguire1
  • Note: for unknown interaction with Solar Hot Water.tsv (which is being tested in project_testing but not in project_national), Water Heater Location.tsv in project_testing cannot be Outside. Otherwise the combination could cause stochastic failures (in idf) that can be re-simulated successfully without change but would cause CI errors.
    ASHRAE IECC Climate Zone 2004
    Vintage ACS
  • Added Geometry Space Combination, a tsv that combines Geometry Building Type RECS, Geometry Building Level MF, Geometry Foundation Type, Geometry Attic Type, and Geometry Garage. It's used to define which combinations are valid and which are not (~80%) so that downstream tsvs (e.g., Water Heater Location and Duct Location) only use the valid options as a dependency and have smaller file size as a result.
  • Note: The same Geometry Space Combination is used for project_testing as project_national. As a result, Geometry Attic Type and Geometry Foundation Type for project_testing are modified to be consistent with the new tsv. The modification affected Mobile Home, which previously can have all attic and foundation types for testing, but is now only assigned Flat Roof (Attic Type=None) and Ambient Foundation Type per project_national. However, the full combinations of attic and foundation types are still being tested by Single-Family Detached.
  • Updated Geometry Garage and Geometry Floor Area Bin (no change as it is a lookup map) to RECS2020
  • Tsvs may have changed due to updating RECS2020 data from v2 to v4 (which contains floor area to enable the update of Geometry Floor Area Bin and Geometry Garage)
  • Added script to automate precomputed buildstocks generation.

Comparison of CI artifact project_national results to Develop (i.e., ./test/base_results/baseline/annual/results* - project_national rows):
water_heater_location_artifact_compare.xlsx

Checklist

Not all may apply:

@lixiliu lixiliu self-assigned this Aug 24, 2023
@lixiliu lixiliu force-pushed the ll/water_heater_location branch 2 times, most recently from c3fc95e to 6a079d6 Compare August 29, 2023 05:41
new tsvs
add 2021 n_buildings_rep count without AK HI
fix error in update_yml_precomputed_files
update CI action config
revert back to openstudio

revert back to using openstudio

add sudo

update config and script
@shorowit shorowit linked an issue Sep 14, 2023 that may be closed by this pull request
Copy link
Contributor

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lixiliu This PR is looking good. Glad to see you were able to get the non deterministic E+ errors (?) sorted out. I have some general questions mainly around the config updates/changes, the recs2009 -> recs2020 changes, and changelog.

.github/workflows/config.yml Outdated Show resolved Hide resolved
.github/workflows/config.yml Outdated Show resolved Hide resolved
project_national/resources/source_report.csv Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Make analysis-tests and integration-tests depend on unit-tests, which help to ensure options_lookup and housing characteristics are structurally consistent o run sampling
@joseph-robertson
Copy link
Contributor

So unit-tests must complete before analysis-tests, which is going to add ~20-30 min to our ci tests. I suppose this is OK, if there's no other way to support the precomputed test file updates.

@lixiliu
Copy link
Contributor Author

lixiliu commented Sep 20, 2023

So unit-tests must complete before analysis-tests, which is going to add ~20-30 min to our ci tests. I suppose this is OK, if there's no other way to support the precomputed test file updates.

@joseph-robertson It's not a hard requirement for pre-computed test file update. It's just that the unit tests would be able to check for structural errors that would cause sampling to hang, so it would be nice to run that before both analysis and integration tests which involve running yamls. It's fine to roll back if that's preferred.


- name: Download formatted options_lookup
uses: actions/download-artifact@v3
with:
path: resources
name: options_lookup

- name: Generate precomputed buildstocks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this! Thank you!

@lixiliu lixiliu merged commit a18bf95 into develop Sep 25, 2023
@lixiliu lixiliu deleted the ll/water_heater_location branch September 25, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Water Heater Installation Location tsv
4 participants