-
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
Update reference wind turbines in the default turbine library #771
Conversation
…ain/floris/turbine_library/nrel_5MW.yaml with extension for before cut in and after cut out. Rename thrust field.
…hrust_coefficient key name.
… (tests need updating yet).
…MW turbine only, will remove prior to merge into v4 branch.
This was previously used for the multidimension turbine, but it has since been consolidated and flatten_dict isn't used
@paulf81 , could you take a quick look at the power and thrust curves in the description above and see if there is anything there of concern? |
floris/turbine_library/nrel_5MW.yaml
Outdated
|
||
# NREL 5MW reference wind turbine. | ||
# Data based on: | ||
# TODO: update URL |
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.
Do we have the correct URL?
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.
The plan is for this to point here: https://github.com/NREL/turbine-models/blob/master/Offshore/NREL_5MW_126_RWT.csv but at the moment, the data hosted at that location is not intuitive (in particular, the thrust coefficient listed does not represent aerodynamic thrust). The plan is to update the data hosted at that location, and I think the URL will stay the same, but I'm hesitant to put it in when it points to unmatched data.
How do you feel about leaving this as a TODO item and possibly creating an issue to remind us to add the URL once the data on https://github.com/NREL/turbine-models/ is 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.
That sounds like a good plan to me
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.
The updated curves have now been provided in the turbine_models repo---thanks @pduff-code!! I've updated the URL.
tests/data/nrel_5MW_v3legacy.yaml
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.
Is this still used?
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.
This is used, along with nrel_5MW_v4converted, to test the utility that builds turbine models based on non-standard data (for example, user supplies Cp curve rather than absolute power curve) in turbine_utilities_unit_test.py. These had been symlinks to their equivalents in the turbine_library, but as those have now been deleted to leave only the updated turbines, I've established the files here so that the test continues to run.
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.
Ok got it. It's worth keeping in mind that there are quite a few NREL 5MW turbine definition files in the repository now, so keeping them in sync and the differences clear will become increasingly difficult. One strategy could be to develop tests that don't use a specific turbine model. In this particular case, the Cp to Power conversion and v3 to v4 conversion don't need the NREL 5MW turbine, they simply need a set of numbers that map to another set of numbers. I'm not suggesting a change here, but we should all keep it in mind.
(base) >>mbp@floris (v4-ms/turbine-library $)$ find . -name "*nrel*.y*ml"
./floris/turbine_library/nrel_5MW.yaml
./tests/data/nrel_5MW.yaml (link to ./floris/turbine_library/nrel_5MW.yaml)
./tests/data/nrel_5MW_custom.yaml
./tests/data/nrel_5MW_v4converted.yaml
./tests/data/nrel_5MW_v3legacy.yaml
./docs/nrel_5MW.yaml (link to ./floris/turbine_library/nrel_5MW.yaml)
./examples/inputs_floating/turbine_files/nrel_5MW_floating.yaml
./examples/inputs_floating/turbine_files/nrel_5MW_floating_fixedtilt15.yaml
./examples/inputs_floating/turbine_files/nrel_5MW_floating_defined_floating.yaml
./examples/inputs_floating/turbine_files/nrel_5MW_floating_fixedtilt5.yaml
./examples/inputs_floating/turbine_files/nrel_5MW_fixed.yaml
tests/data/nrel_5MW_v4converted.yaml
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.
This can probably be called "nrel_5MW" or also symlink to the turbine library
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.
Unfortunately, that doesn't work---nrel_5MW_v4converted.yaml has data matching the v3legacy.yaml model, which is slightly different to the (new) nrel_5MW.yaml in turbine_library because we have updated the data slightly. In the PR description, the red curve in the first plot is formed by turbine_library/nrel_5MW.yaml, while the black and gray curves are from nrel_5MW_v3legacy.yaml and nrel_5MW_v4converted.yaml, respectively.
An option is to construct a v3-like turbine (but that does not exactly match what is in v3) to compare the nrel_5MW.yaml that is in turbine_library to.
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.
I think this is a good idea. Otherwise, we'll have multiple NREL 5MW definitions within the same version of floris.
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.
I'll try to make that change today.
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.
@rafmudaf I've now updated that by removing nrel_5MW_v4converted.yaml and nrel_5MW_v3legacy.yaml from tests/data and adding data for a v3-type turbine that matches the updated nrel_5MW data to SampleInputs
on conftest.py. Let me know if you think that's an improvement.
There is still a symlink to the NREL_5MW.yaml in tests/data, alongside a turbine model NREL_5MW_custom.yaml. I believe these are used in farm_unit_test.py, but are not touched by this PR. (Well, not directly---the data for the NREL_5MW.yaml symlink does change because it's the data on its target changes). Because I did not updated NREL_5MW_custom.yaml, it is still in the form of a v3 turbine.
…to data disconnect; update defaults in build_cosine_loss_turbine_dict to match updated NREL 5MW.
…tch v4 NREL 5MW model and pass tests.
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.
Thanks @misi9170 , I went through the code and ran the examples, all good on my end!
Description
In converting from Cp to power curves as the standard for v4, we also noted that the data for some of the models in turbine library (most notably, the NREL 5MW) are out of date. This pull request would update that data so that the turbine models now follow those specified here and here*.
Legacy FLORIS v3 turbines are first archived in the subdirectory legacy_v3 for clarify, and then removed. Similarly, the convert_turbine_v3_to_v4 utility is used to generate turbines exactly matching the v3 turbines, which are archived in converted_from_v3 and then deleted. Finally, the (deleted) subdirectory updated_for_v4 contained turbines whose data matches the updated versions in turbine_library/.
Affected areas
Supersedes #767, which will be closed. Should be merged after #770.
Changes to the turbine power and thrust curves
Updating the data has slightly changed the power and thrust curves of the NREL 5MW, IEA 10MW, and IEA 15MW turbines (the x_20MW turbine has been archived and has not been brought forward at this stage for lack of a public data source that we could easily find).
The new and previous power curves are as follows:
These can be generated by running the following script from within the turbine_library directory:
Other notes
* The current definition of the NREL 5MW reference turbine specified a Ct value that is not the aerodynamic thrust coefficient. We're working with @pduff-code to update that.