Feature add: integration with turbine-models library for wind simulations #435
Conversation
…ers to windconfig
| # if multiple hub height options are available | ||
| if isinstance(turbine_specs["hub_height"],list): | ||
| # check for hub height in wind_plant | ||
| if isinstance(wind_plant,object) and wind_plant is not None: |
There was a problem hiding this comment.
We should be checking for one of the two options that are specified in the docstring, not object. Once this is done, the check for is not None can be removed.
| if wind_plant.config.hub_height is not None: | ||
| if any(float(k) == float(wind_plant.config.hub_height) for k in turbine_specs["hub_height"]): | ||
| hub_height = wind_plant.config.hub_height | ||
| logger.info(f"multiple hub height options available for {turbine_name} turbine. Setting hub height to WindConfig hub_height: {hub_height}") |
There was a problem hiding this comment.
For longer messages like the ones in this function, could you reformat them to be like the following? The specific line length doesn't matter too much, but a few of these logging messages are well past the 122 character ruler on my monitor, so even with a large monitor in full screen mode it requires a horizontal scroll when viewing two files at once.
msg = (
f"multiple hub height options available for {turbine_name} turbine."
f" Setting hub height to WindConfig hub_height: {hub_height}"
)
logger.info(msg)
| if isinstance(wind_plant,object) and wind_plant is not None: | ||
|
|
||
| # check if hub_height was put in WindConfig | ||
| if wind_plant.config.hub_height is not None: |
There was a problem hiding this comment.
This block could be simplified to the following format. Just a note to use the turbine_hub_height throughout in place of the dictionary call. I might also just suggest using name and hub_height because this is a turbine checking function, so the context is fully there already.
# NOTE: this line goes after the turbine_name definition
turbine_hub_height = turbine_specs["hub_height"]
if isinstance(turbine_hub_height,list):
# check for hub height in wind_plant
if isinstance(wind_plant,object) and wind_plant is not None:
if (hub_height := wind_plant.config.hub_height) is not None:
if any(k == hub_height for k in turbine_hub_height):
logger.info("...")
return hub_height
if (hub_height := option2.hub_height) is not None:
if any(k == hub_height for k in turbine_hub_height):
logger.info("...")
return hub_height
... # the other checks
| return power_thrust_table | ||
|
|
||
|
|
||
| def check_hub_height(turbine_specs,wind_plant): |
There was a problem hiding this comment.
Can you update the function definition lines in this file to all have a space after the commas?
| else: | ||
| raise ValueError(f"turbine {turbine_name} is missing some data, please try another turbines") | ||
| return turbine_dict |
There was a problem hiding this comment.
The return line should replace the else statement and the last line of the function should be the error raising.
| else: | ||
| raise ValueError(f"turbine {turbine_name} is missing some data, please try another turbines") | ||
| return turbine_dict |
There was a problem hiding this comment.
Same comment here about the control flow.
| distributed_turbines_dict = t_lib.turbines(group = "distributed") | ||
| distributed_turbines = [v for k,v in distributed_turbines_dict.items()] |
There was a problem hiding this comment.
distributed_turbines = list(t_lib.turbines(group="distributed").values())
| lbw_turbines_dict = t_lib.turbines(group = "onshore") | ||
| lbw_turbines = [v for k,v in lbw_turbines_dict.items()] |
There was a problem hiding this comment.
Some one-liner as the above function.
| osw_turbines_dict = t_lib.turbines(group = "offshore") | ||
| osw_turbines = [v for k,v in osw_turbines_dict.items()] |
There was a problem hiding this comment.
Same one-liner as the above functions.
| valid_name = False | ||
| best_match = "" | ||
| max_match_ratio = 0.0 | ||
| for turb_group in t_lib.groups: |
There was a problem hiding this comment.
Should the group be specified as a function argument? There are two possible scenarios I can see where that would be helpful: 1) the user input aligns more with a different group's naming and now they have an offshore turbine for a land-based project (or similar mixed up scenario), and 2) it should be known by the time the model has gotten this far into validation which scenario is being run (unless I'm misunderstanding)
| @@ -49,7 +49,8 @@ dependencies = [ | |||
| "utm", | |||
| "pyyaml-include", | |||
| "profast", | |||
| "NREL-rex" | |||
| "NREL-rex", | |||
| "turbine_models @ git+https://github.com/NREL/turbine-models.git", | |||
There was a problem hiding this comment.
Tagging a git version will render this project unable to be updated on PyPI, so this will either have to get converted to a real package (if there are funds I can help you and Patrick streamline this process, if helpful) or be a recommended git installation for the user.
| t_lib = Turbines() | ||
| valid_name = False | ||
| best_match = "" | ||
| max_match_ratio = 0.0 |
There was a problem hiding this comment.
This seems to be a really low bar to sucess for finding a match.
There was a problem hiding this comment.
I've agree - I've removed the usage of this function in the code. Instead of using this function, it will just print all the available turbine names and raise a value error.
| if any(turb.lower()==turbine_name.lower() for turb in turbines_in_group.values()): | ||
| valid_name = True | ||
| return turbine_name | ||
| elif any(turbine_name.lower() in turb.lower() for turb in turbines_in_group.values()): |
There was a problem hiding this comment.
Because there is a return directly above, this elif should be an if. I would also change the following since it's duplicated in the next line:
turbine_options = [turb for turb in turbines_in_group.values() if turbine_name.lower() in turb.lower()]
if len(turbine_options) == 1:
return turbine_options[0]
if len(turbine_options) > 1:
... # The code with the else statement (lines 62-66)There was a problem hiding this comment.
same comment about this function not being used anymore.
| if match_ratio>max_match_ratio: | ||
| best_match = str(turb) | ||
| max_match_ratio = max(match_ratio,max_match_ratio) | ||
| if valid_name: |
There was a problem hiding this comment.
This variable is only assigned at the top to False, and just before a return statement, so there is no instance in which this if statement is True.
There was a problem hiding this comment.
function removed from workflow
| match_ratio = SequenceMatcher(None,turbine_name.lower(), turb.lower()).ratio() | ||
| if match_ratio>max_match_ratio: | ||
| best_match = str(turb) | ||
| max_match_ratio = max(match_ratio,max_match_ratio) | ||
|
|
||
| else: | ||
| for turb in turbines_in_group.values(): | ||
| match_ratio = SequenceMatcher(None,turbine_name.lower(), turb.lower()).ratio() | ||
| if match_ratio>max_match_ratio: | ||
| best_match = str(turb) | ||
| max_match_ratio = max(match_ratio,max_match_ratio) |
There was a problem hiding this comment.
I'm not really convinced that we should be hand holding the user to manage typos other than capitalization and extra spaces because this opens a can of worms of why here and not there. I'm interested to know others' thoughts on this matter because I may be out of touch, but this feels a bit out of scope for this model's goals.
There was a problem hiding this comment.
function removed from workflow
| else: | ||
| return best_match | ||
|
|
||
| def check_turbine_library_for_turbine(turbine_name:str): |
There was a problem hiding this comment.
This is functionally the same code as above, just without the SequenceMatcher methodology, or substring checking methodology. Personally, this is the version I think is most relevant, though I think we should also be passing which group it comes from.
There was a problem hiding this comment.
I agree - function removed from workflow.
hopp/tools/resource/wind_tools.py
Outdated
| speeds = data[:, idx_ws[0]] | ||
| wind_dirs = data[:, idx_wd[0]] | ||
| return speeds, wind_dirs |
There was a problem hiding this comment.
These should now be in the top-most if-block. The else attached to that if can now be removed with the code inside that block dedented.
| @@ -1,6 +1,6 @@ | |||
| import os | |||
| import yaml | |||
|
|
|||
| import dill | |||
There was a problem hiding this comment.
This isn't a current dependency of HOPP, though multiprocessing-on-dill is. Could you add dill since it's clear it should have already been there?
hopp/utilities/utilities.py
Outdated
| else: | ||
| already_exists = True |
There was a problem hiding this comment.
This could be simplified by setting the variable as True just before the if block.
hopp/utilities/utilities.py
Outdated
|
|
||
| with open(filename, 'w+') as file: | ||
| yaml.dump(data, file,sort_keys=False,encoding = None,default_flow_style=False) | ||
| return filename |
There was a problem hiding this comment.
Is there a reason why a file name is being returned? It doesn't seem to be being modified, so it feels redundant.
| def dump_data_to_pickle(data,filepath): | ||
| with open(filepath,"wb") as f: | ||
| dill.dump(data,f) |
There was a problem hiding this comment.
I'm not a huge fan of pickling objects, could you just confirm if this is for an intermediary step in the process?
There was a problem hiding this comment.
this isn't used or integrated into the workflow at the moment, I just like the option to save objects as pickles for debugging or replicating purposes.
| with open(filepath,"wb") as f: | ||
| dill.dump(data,f) | ||
|
|
||
| def load_dill_pickle(filepath): |
There was a problem hiding this comment.
This naming will live rent free in my head whenever I open a pickle jar now.
tests/hopp/test_custom_financial.py
Outdated
| @@ -8,7 +8,6 @@ | |||
| from tests.hopp.utils import create_default_site_info, DEFAULT_FIN_CONFIG | |||
| import copy | |||
| import numpy as np | |||
|
|
|||
There was a problem hiding this comment.
Can you put this deleted line back?
| power_curve_kw = np.array(turbine_specs["power_curve"]["power_kw"].to_list()) | ||
| cp_curve = np.array(turbine_specs["power_curve"]["cp"].to_list()) | ||
| power_curve_kw = np.where(power_curve_kw<0,0,power_curve_kw) | ||
| power_curve_kw = np.where(power_curve_kw>turbine_specs["rated_power"],turbine_specs["rated_power"],power_curve_kw).tolist() | ||
| cp_curve = np.where(cp_curve<0,0,cp_curve).tolist() |
There was a problem hiding this comment.
Can you make it so the power_curve_kw and cp_curve methods are ordered as one, and then other since they aren't reliant on each other? Otherwise, thanks for cleaning this up so this large control flow block so it' easier to follow, the hybrid approach is clearer than my original suggestion too!
Turbine-models library integration!
Integrate the ability to simulate wind turbines available in the turbine-models package with both PySAM and FLORIS wind simulation options!
PR Checklist
RELEASE.mdhas been updated to describe the changes made in this PRdocs/files are up-to-date, or added when necessaryRelated issues
Issue #428
Impacted areas of the software
Modified workflow files and functions
hopp/simulation/technologies/wind/wind_plant.pyWindConfig:turbine_nameandturbine_managementWindPlant:__attrs_post_init__(): updated to callinitialize_pysam_wind_turbine()if using pysaminitialize_pysam_wind_turbine(): new function to handle pysam-specific attribute setting, set turbine model from turbine-models library, and redownload resource data if discrepancy in turbine-height and wind resource data heights.num_turbinessetter: modified so if using "custom" layout, it doesnt throw user-warning upon initialization (which would happen previously when called from__attrs_post_init__() method)hopp/simulation/technologies/wind/floris.pyclassFlorisinitialize_from_floris(): modified to callinitialize_wind_turbine()and to re-download resource data if discrepancy in turbine hub-height and wind resource data heights.initialize_wind_turbine(): new function to set turbine model infloris_configupon Floris initializationupdate_wind_turbine(): new function to update turbine model after Floris has been initialized.hopp/tools/resource/wind_tools.pyparse_resource_data: modified to not average resource data for multiple heights if hub-height is equal to lower or upper resource height.New files and functions
hopp/tools/design/wind/: new files and functionsfloris_helper_tools.pycheck_output_formatting(): recursive method to convert types in a dictionary to human readable format if its exported to a yaml filewrite_floris_layout_to_file(): function to write a wind farm layout to FLORIS-compatible yaml file format.write_turbine_to_floris_file(): function to write turbine model to FLORIS-compatible yaml file format.check_floris_library_for_turbine(): returns true if turbine_name is in the FLORIS internal turbine library, otherwise returns falseload_turbine_from_floris_library(): loads a turbine from the FLORIS internal turbine library.check_libraries_for_turbine_name_floris(): checks FLORIS internal turbine library and turbine-models library for turbine name, returns a FLORIS-compatible turbine model dictionary if its a valid turbine name, returns a string if the turbine is not found that can be used in a UserWarning message.power_curve_tools.py:plot_power_curve(): plot Cp and Ct curve across wind speeds (created by @genevievestarke)pad_power_curve(): if a power-curve is only defined from cut-in to cut-out wind speeds, this pads the power-curve from 0 m/s to 50 m/s wind speed so no errors are thrown in wind simulations.calculate_cp_from_power(): Calculate power coefficient curve (Cp) from power curve.calculate_power_from_cp(): Calculate power curve from power coefficient curve (Cp).estimate_thrust_coefficient(): Calculate thrust coefficient curve (Ct) from power coefficient curve (Cp). (created by @genevievestarke)turbine_library_interface_tools.pyextract_power_curve(): gets Power, Cp, and Ct curve for turbine and formats for simulations.check_hub_height(): if turbine from turbine-models library has multiple hub-height options, checks the other places a user may have specified a specific hub-height or sets to median hub-height.get_pysam_turbine_specs(): Load turbine data from turbine-models library in PySAM-compatible format.get_floris_turbine_specs(): Load turbine data from turbine-models library in FLORIS-compatible format.turbine_library_tools.pyintegrated functions:check_turbine_name(): returns a valid turbine name in turbine-library that is best-match to user-provided turbine name.check_turbine_library_for_turbine(): returns True if turbine is found in turbine library, returns false otherwiseNew tests
All new tests are in
tests/hopp/test_wind_design_tools.py: unit tests for floris integration toolstest_floris_library_tools_for_valid_floris_turbine(): unit test forcheck_floris_library_for_turbineandload_turbine_from_floris_libraryinfloris_hepler_toolswith a valid floris-library turbine nametest_floris_library_tools_for_invalid_floris_turbine(): unit test forcheck_floris_library_for_turbineandload_turbine_from_floris_libraryinfloris_hepler_toolswith an invalid floris-library turbine nametest_floris_turbine_loader_valid_floris_turbine(): unit test forcheck_libraries_for_turbine_name_floris()infloris_hepler_toolstest_floris_turbine_loader_multi_hub_height_turbine(): functions as a unit test forcheck_hub_height()inturbine_library_interface_tools.pytest_floris_turbine_loader_single_hub_height_turbine(): functions as a unit test forcheck_hub_height()inturbine_library_interface_tools.pytest_turbine_models_interface.pytest_turbine_library_tools_for_valid_turbine_name(): unit test forcheck_turbine_nameandcheck_turbine_library_for_turbineinturbine_library_tools.pytest_turbine_library_tools_for_invalid_turbine_name(): unit test forcheck_turbine_nameandcheck_turbine_library_for_turbineinturbine_library_tools.pytest_floris_nrel_5mw(): tests WindPlant integration for runnning floris with a turbine from the FLORIS internal turbine librarytest_floris_nrel_5mw_hopp(): tests HOPP integration for runnning floris with a turbine from the FLORIS internal turbine librarytest_floris_NREL_5MW_RWT_corrected_hopp(): tests HOPP integration for runnning floris with a turbine from the turbine-models library.test_pysam_NREL_5MW_RWT_corrected_hopp(): tests HOPP integration for runnning pysam with a turbine from the turbine-models library.Other files modified:
pyproject.toml: modified to include turbine models library in dependenciesRELEASE.md: added in feature infoREADME.md: changed language to include "distributed"Additional supporting information
Test results, if applicable