-
Notifications
You must be signed in to change notification settings - Fork 156
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
Tum misalignment model #832
base: develop
Are you sure you want to change the base?
Conversation
… use with TUMLossTurbine operation model).
…all dummy misalignment in case both yaw and tilt are 0. This was causing a division by zero in the tests.
…n calling TUMLossTurbine.control_trajectory() so that rated power stays independent from air_density
…pears identical to rotor_velocity_yaw_cosine_correction.
This reverts commit 0ced9d2.
Hi @sTamaroTum , Thank you for your patience waiting for us to work through this pull request. I have finally been able to spend some time working on it, and have made various changes (while trying to ensure that I have not changed the outputs at all---see my comment about regression tests below). Main changesThe main changes I've made while working through your code are the following:
TestsI also separated out tests for the model into a new file tests/tum_operation_model_unit_test.py. To run the tests, you'll need to call DocumentationIt would be good to add a bit of high-level documentation on the TUM model. I will put a placeholder in imminently, but it would be good to have you add some extra there for people coming to FLORIS and wanting to know more about the model and how it works. Other notes
Next stepsAt this stage, it would be great to get your input on the following:
In the meantime, I have the following action items:
Once we are happy that the code is final, @rafmudaf and I will take a last pass through it to see if there are any other final changes needed before we merge. Again, my apologies on how long this took for us to get to---I hope that we'll be able to move it forward more rapidly now that I've been able to spend some time with it! Misha |
…rmation; add tests for range of wind speeds and grid points.
Just a few small updates to my comment above:
|
@@ -29,31 +29,44 @@ operation_model: 'cosine-loss' | |||
### | |||
# Parameters needed to evaluate the power and thrust produced by the turbine. | |||
power_thrust_table: | |||
### Power thrust table parameters | |||
### General parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a heads up that these comments are used to automatically generate this page in the docs: https://nrel.github.io/floris/input_reference_turbine.html. The structure (number of hashes) and location relative to the key/value pairs does matter. If you haven't it's worthwhile to build the docs locally and see the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, my bad, thanks---I'll do that and adjust as needed
floris/type_dec.py
Outdated
@@ -59,7 +59,7 @@ def floris_array_converter(data: Iterable) -> np.ndarray: | |||
raise TypeError(e.args[0] + f". Data given: {data}") | |||
return a | |||
|
|||
def floris_numeric_dict_converter(data: dict) -> dict: | |||
def floris_numeric_dict_converter(data: dict, allow_strings=False) -> dict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider an "allow_any" flag instead of specifically allowing strings. Also, consider not using this function if you want to convert a dictionary that isn't all numbers. And also there's a new argument added but the docstring isn't updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm now also realizing it may make more sense to load the data stored in the string ahead of time, which would avoid this issue. I'm going to see if that'll work
tests/conftest.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More and more, having yet another version of the inputs defined in this standalone class only for testing seems wasteful. I'm suggesting it be addressed here, but just pointing it out.
def test_TUMLossTurbine(): | ||
|
||
# NOTE: These tests should be updated to reflect actual expected behavior | ||
# of the TUMLossTurbine model. Currently, match the CosineLossTurbine model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget about this one!
…l calls. Revert type_dec changes.
…els; may remove from iea10mw, nrel5mw subsequently.
TUM misalignment model
This is a proposal to include the misalignment model developed by TUM to floris v4. The methodology is based on the preprint available currently at https://wes.copernicus.org/preprints/wes-2023-133/ .
The model is coded into simulation/turbine/operation_models.py . It consists of the usual power(), thrust(), axial_induction() functions. There are two extra functions called compute_local_vertical_shear() and control_trajectory().
The model requires a few new inputs that were added to the power_thrust_table in the yaml file. It also requires the cp-pitch-tsr curves for the turbine model considered. An .npz file is added to the turbine_library folder, as well as a file iea_3mw.yaml file ready for use with the model. This file refers to the IEA 3 MW reference turbine by Bortolotti et al. (2019). In the future, more wind turbine models could be added.
The model accepts power_setpoints inputs in watts to simulate derating.
The model allows also to obtain the pitch and tip speed ratio of the wind turbine. Right now the code is printing the values of tip speed ratio and pitch to screen. This could be removed for clarity, but get_pitch() and get_tsr() functions could be coded (?).
Related issue
Impacted areas of the software
simulation/turbine/operation_models.py
Additional supporting information
The model seem to work well with the examples, but it crashes at wind speeds higher than 18 m/s, because the operating tip-speed-ratio gets too low.
In the tum model, the tilt is negative when tower clearance is created. This seems conflicting with floris convention, so there is a sign change at the start of the power(), thrust() and axial_induction() functions.
Sorry if something is wrong, this is the first time I work with github.
Test results, if applicable