You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@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.
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
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 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 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@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_typefield on theturbine_definitionsentry dictionaries. It is not immediately clear to me why this should be the case---the current operation is that if multiple entries of theturbine_definitionslist all have the sameturbine_typekey, then only the first list entry is used.To see this, you can comment out the lines
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_typekeys need to be unique? This also came up in the example code I wrote for #851, where I had to rename theturbine_typein order for FLORIS to recognize different hub heights.