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

Use ETS heat pump COP from GMT sys-params #32

Merged
merged 16 commits into from
Aug 7, 2024
Merged

Use ETS heat pump COP from GMT sys-params #32

merged 16 commits into from
Aug 7, 2024

Conversation

vtnate
Copy link
Collaborator

@vtnate vtnate commented Jun 20, 2024

Any background context you want to provide?

We were unintentionally not allowing users to change ETS heat-pump COP values. That is now available

What does this PR accomplish?

  • Read cop_c and cop_h values from a sys-param file
    • Enabled possibility of reading any ETS data from sys-param file in the future
  • Change 2 buildings' ETS COP values in the 13-building upstream sys-param file for demonstration/validation purposes
  • Fix a few typos
  • Update all sys-param files to have fifth-gen ETSs attached to the buildings. This was a copy/paste mistake originally

How should this be manually tested?

  • Run the 13-building upstream demo file using the CLI on the command line
  • Review the terminal output of final ets_data: for cop_c and cop_h.
    • While most will be 3.5 & 2.5 respectively, there should also be a 3.4 & 2.6, as well as a 3.0 & 3.0, demonstrating that TN is reading from the sys-param file and applying the values to the ETS object correctly.

What are the relevant tickets?

Screenshots (if appropriate)

@vtnate vtnate added the enhancement New feature or request label Jun 20, 2024
@vtnate vtnate self-assigned this Jun 20, 2024
@vtnate vtnate marked this pull request as ready for review June 24, 2024 16:04
@vtnate vtnate requested a review from mitchute June 24, 2024 16:05
@vtnate vtnate changed the title Expose district default heat pump COP Use ETS heat pump COP from GMT sys-params Jul 30, 2024
Copy link
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

Looks good, but it would be good to add a test here. I merged in multi-ghe locally and ran tests, and they all passed. I think once we have a test for the changes in this branch, I'm good.

@vtnate
Copy link
Collaborator Author

vtnate commented Aug 2, 2024

Looks good, but it would be good to add a test here. I merged in multi-ghe locally and ran tests, and they all passed. I think once we have a test for the changes in this branch, I'm good.

Ahh, right. I have the json file to show the changes, but I never actually made an assertion for it. I'll make that happen. Thanks Matt!

@vtnate vtnate merged commit e4d6a16 into main Aug 7, 2024
9 checks passed
@vtnate vtnate deleted the expose-cop branch August 7, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants