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

roof/window measure updates and combining with HP-RTU measure #276

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

JanghyunJK
Copy link
Contributor

@JanghyunJK JanghyunJK commented Jan 24, 2025

Pull request overview

  • roof upgrade measure update

    • most functionality moved to ComStock\resources\measures\upgrade_env_roof_insul_aedg\resources\upgrade_env_roof_insul_aedg.rb
  • window upgrade measure update

    • most functionality moved to ComStock\resources\measures\upgrade_env_new_aedg_windows\resources\upgrade_env_new_aedg_windows.rb
  • HP-RTU measure update

    • the "exact same" two files above are saved under this measure folder: ComStock\resources\measures\upgrade_hvac_add_heat_pump_rtu\resources
    • to provide the same functionality of roof/window upgrade measures inside of HP-RTU measure
    • but to use HP-RTU applicability logic as the sole applicability logic
    • minimum unit test added to check if window/roof measures are implemented or not implemented correctly
  • so, based on the suggestion in this PR,

    • whenever someone makes changes to either of these two files below,
      ComStock\resources\measures\upgrade_env_roof_insul_aedg\resources\upgrade_env_roof_insul_aedg.rb
      ComStock\resources\measures\upgrade_env_new_aedg_windows\resources\upgrade_env_new_aedg_windows.rb
    • then the same change should happen here:
      ComStock\resources\measures\upgrade_hvac_add_heat_pump_rtu\resources
    • and vise versa

Pull Request Author

This pull request makes changes to (select all the apply):

  • Documentation
  • Infrastructure (includes apptainer image, buildstock batch, dependencies, continuous integration tests)
  • Sampling
  • Workflow Measures
  • Upgrade Measures
  • Reporting Measures
  • Postprocessing

Author pull request checklist:

  • Tagged the pull request with the appropriate label (documentation, infrastructure, sampling, workflow measure, upgrade measure, reporting measure, postprocessing) to help categorize changes in the release notes.
  • Added tests for new measures
  • Updated measure .xml(s)
  • Register values added to comstock_column_definitions.csv
  • Both options_lookup.tsv files updated
  • 10k+ test run
  • Change documentation written
  • Measure documentation written
  • ComStock documentation updated
  • Changes reflected in example .yml files
  • Changes reflected in README.md files
  • Added 'See ComStock License' language to first two lines of each code file
  • Implements corresponding measure tests and indexing path in test/reporting_measure_tests.txt, test/workflow_measure_tests.txt, or test/upgrade_measure_tests.txt
  • All new and existing tests pass the CI

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: data and method additions, changes, tests
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • Reviewed change documentation
  • Ensured code files contain License reference
  • Results differences are reasonable
  • Make sure the newly added measures has been added with tests and indexed properly
  • CI status: all tests pass

@JanghyunJK JanghyunJK added the upgrade measure PR improves or adds upgrade measures label Jan 24, 2025
@JanghyunJK JanghyunJK changed the title [WIP] roof/window measure updates and combining with HP-RTU measure roof/window measure updates and combining with HP-RTU measure Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upgrade measure PR improves or adds upgrade measures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant