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

Defaults standardization #10

Merged
merged 3 commits into from
Oct 4, 2024
Merged

Defaults standardization #10

merged 3 commits into from
Oct 4, 2024

Conversation

atpham88
Copy link
Collaborator

@atpham88 atpham88 commented Mar 26, 2024

About

This PR updates REopt's default GHP/GHX parameters to match with defaults from ResStock, ComStock and URBANopt as part of an effort to standardize defaults across NREL models.

Updated Defaults

Default Defaults values changed Notes
borehole_depth_ft 400.0 ft => 443.0 ft ResStock = 152m, ComStock, URBANopt = 135m = 443 ft. Goes with ComStock & URBANopt
ghx_header_depth_ft 4.0 ft => 6.6 ft ResStock, ComStock, URBANopt = 2m = 6.56168 ft
borehole_diameter_inch 5.0 in => 6.0 in ResStock, ComStock, and URBANopt = 6 in
ghx_pipe_thermal_conductivity_btu_per_hr_ft_f 0.25 => 0.23 ComStock & URBANopt = 0.4 W/m-K = is 0.23127039296 BTU/ft-F
ghx_shank_space_inch 2.5 in => 1.27 in ResStock = 0.0246m = 0.97 in, ComStock & URBANopt = 0.0323m = 1.27 in. Goes with ComStock & URBANopt
grout_thermal_conductivity_btu_per_hr_ft_f 1.0 => 0.75 ResStock, ComStock, URBANopt = 1.3 W/m-K = 0.75 BTU/ft-F

Test Case

GHP module was run for a site in Alaska before and after the changes:

Output Before the changes After the changes
heat_pump_configuration "WSHP" "WSHP"
peak_heating_heatpump_thermal_ton 121.808 121.808
number_of_boreholes 52 64
length_boreholes_ft 393.2 396.2
yearly_total_electric_consumption_kwh 98113.6 95848.6

@atpham88 atpham88 marked this pull request as ready for review March 26, 2024 17:09
@atpham88 atpham88 requested a review from Bill-Becker March 26, 2024 17:09
@Bill-Becker
Copy link
Collaborator

@atpham88 I think you can remove the "old value = X" in the comments. I think when we merge this, the changes may automatically get merged into the main version of the REopt API because of the way we load this package in. So, we need to coordinate merging this with the web tool team to make sure they reflect the updated defaults.

@atpham88
Copy link
Collaborator Author

@Bill-Becker Thanks Bill. I removed the old values in comment. Should I remove the ResStock, ComStock, URBANopt values as well?

@Bill-Becker
Copy link
Collaborator

@Bill-Becker Thanks Bill. I removed the old values in comment. Should I remove the ResStock, ComStock, URBANopt values as well?

I think if we're changing the values to almost match the other tools, we don't need to state the other tool's value. However, if they are significantly different (still), it might be useful to say what the other tools are using.

Also, I forgot to mention that the API has defaults for ghpghx_inputs in the field definitions, so those should be updated to match these updates. It would be good for you to cross-check other defaults between GhpGhx.jl and here as well, even if you didn't change them:
https://github.com/NREL/REopt_API/blob/master/ghpghx/models.py

@atpham88
Copy link
Collaborator Author

@Bill-Becker Sorry I forgot about this for a while. Getting back to it now per Kathleen's email. I checked the GHX defaults in the API and they all match with the ones currently in GhpGhx. I updated these values in the API as well in this PR NREL/REopt_API#604

@atpham88 atpham88 merged commit fb3d8f5 into main Oct 4, 2024
@atpham88 atpham88 deleted the defaults-standardization branch October 29, 2024 22:18
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.

2 participants