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

[BUGFIX] hardcoded reset to first turbine type in set_operation_model #856

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

misi9170
Copy link
Collaborator

@misi9170 misi9170 commented Mar 27, 2024

@Bartdoekemeijer pointed out that FlorisModel.set_power_thrust_model(), introduced in #840, seems to introduce a bug that sets all turbines to the same type.

This PR fixes that bug, and also allows users to specify different power_thrust_models for different turbines in the farm.

However, in order to do so, it must change the name of the turbine_type field on the turbine_definitions entry dictionaries. It is not immediately clear to me why this should be the case---the current operation is that if multiple entries of the turbine_definitions list all have the same turbine_type key, then only the first list entry is used.

To see this, you can comment out the lines

            turbine_type_list[tindex]["turbine_type"] = (
                turbine_type_list[tindex]["turbine_type"]+"_"+power_thrust_model[tindex]
            )

in this PR, which will cause the test to fail because both turbines will be "simple-derating".
@rafmudaf , @bayc , do you know why it's the case that the turbine_type keys need to be unique? This also came up in the example code I wrote for #851, where I had to rename the turbine_type in order for FLORIS to recognize different hub heights.

@misi9170 misi9170 added bug Something isn't working v4 Focus of FLORIS v4 labels Mar 27, 2024
@misi9170 misi9170 requested review from bayc and rafmudaf April 1, 2024 15:57
@misi9170 misi9170 changed the title [BUGFIX] hardcoded reset to first turbine type in set_power_thrust_model [BUGFIX] hardcoded reset to first turbine type in set_operation_model Apr 4, 2024
@misi9170 misi9170 merged commit 397d93c into NREL:v4 Apr 4, 2024
8 checks passed
@misi9170 misi9170 deleted the bugfix/set_power_thrust_model branch April 4, 2024 15:39
@bayc
Copy link
Collaborator

bayc commented Apr 4, 2024

@misi9170 I believe we set it up that way to facilitate grabbing the turbine definitions from the turbine library. You can see here (https://github.com/NREL/floris/blob/v4/floris/core/farm.py#L113) that we build a turbine definition cache, and each turbine definition needs a unique name so that it can be mapped across all the turbines that share that type.

@bayc
Copy link
Collaborator

bayc commented Apr 4, 2024

@misi9170 I believe we set it up that way to facilitate grabbing the turbine definitions from the turbine library. You can see here (https://github.com/NREL/floris/blob/v4/floris/core/farm.py#L113) that we build a turbine definition cache, and each turbine definition needs a unique name so that it can be mapped across all the turbines that share that type.

@misi9170 , ok, digging in further, it should be the case that turbines can have the same turbine_type, and shouldn't be required to be unique, so will need to dig further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v4 Focus of FLORIS v4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants