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

EUSS HP-RTU measure updates #233

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

Conversation

JanghyunJK
Copy link
Contributor

@JanghyunJK JanghyunJK commented Oct 16, 2024

Pull request overview

@ChristopherCaradonna @carlobianchi89 this is the latest HPRTU measure PR!

This became a very complicated change including multiple sub-PRs and multiple updates on the same thing. But below are the summary of changes:

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/measure_tests.txt or/and test/resource_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 self-assigned this Oct 16, 2024
@JanghyunJK JanghyunJK mentioned this pull request Oct 16, 2024
29 tasks
@JanghyunJK JanghyunJK added upgrade measure PR improves or adds upgrade measures reporting measure PR improves or adds reporting measures labels Oct 16, 2024
@mdahlhausen mdahlhausen changed the base branch from develop to main October 18, 2024 19:17
@@ -2746,9 +2746,9 @@ def run(runner, user_arguments)
coil_heating = airloopunisys.heatingCoil.get
# get appropriate cycling ratio timeseries depending on coil type
ts_cycling_ratio = nil
if coil_cooling.to_CoilCoolingDXTwoSpeed.is_initialized
if coil_heating.to_CoilCoolingDXTwoSpeed.is_initialized
Copy link
Contributor

@ChristopherCaradonna ChristopherCaradonna Oct 18, 2024

Choose a reason for hiding this comment

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

@JanghyunJK to_CoilCoolingDXTwoSpeed -> should these be to_Coil**Heating**DXTwoSpeed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reporting measure PR improves or adds reporting measures upgrade measure PR improves or adds upgrade measures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants