-
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
Clarify turbine definition terms #815
Conversation
@@ -180,8 +180,8 @@ class CosineLossTurbine(BaseOperationModel): | |||
""" | |||
Static class defining an actuator disk turbine model that may be misaligned with the flow. | |||
Nonzero tilt and yaw angles are handled via cosine relationships, with the power lost to yawing | |||
defined by the pP exponent. This turbine submodel is the default, and matches the turbine | |||
model in FLORIS v3. | |||
defined by the cosine_loss_exponent_yaw. This turbine submodel is the default, and matches the |
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 way this sentence reads is unclear. I suggest either rephrasing to plane english such as "defined the the corresponding cosine loss exponent for yaw and tilt".
floris/simulation/turbine/turbine.py
Outdated
@@ -130,7 +130,7 @@ def power( | |||
""" | |||
# TODO: Change the order of input arguments to be consistent with the other | |||
# utility functions - velocities first... | |||
# Update to power calculation which replaces the fixed pP exponent with | |||
# Update to power calculation which replaces the fixed cosine_loss_exponent_yaw exponent with |
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.
Similar to in the operation models module, this might be more clear in plane english
@@ -57,8 +57,8 @@ | |||
valid_properties = [ | |||
"generator_efficiency", |
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 valid?
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.
Yes---this one property (generator_efficiency
) is still needed for the conversion of a v3 turbine to a v4 turbine, so is kept at this stage. However, the output v4 turbine yaml will not contain generator_efficiency
(it is used in build_cosine_loss_turbine_dict()
and then discarded).
Turbine definition yamls contain a couple of terms that were unclear and needed clarification. This discussion began with my misunderstanding of the key
pT
(see Further discussion needed on #770), but also extends topT
and the now-unusedgenerator_efficiency
key (following #765).Accordingly, this PR:
pP
tocosine_loss_exponent_yaw
on the turbine definition yamls and throughout the codepT
tocosine_loss_exponent_tilt
on the turbine definition yamls and throughout the codegenerator_efficiency
key on the turbine definition yamls, and adds comments to turbine models in the turbine_library to indicate the generator efficiencies used in generating the absolute power curves.There is one loose end in turbine.py---the multidimensional cp ct table still specifies a Cp value rather than an absolute power (again, see Further discussion needed on #770), so there is still a need for generator efficiency. For now, I have hardcoded the efficiency of the NREL 5MW, which enables the tests to pass. However, @bayc is currently in the process of switching the multidimensional turbine definitions to an absolute power rather than a cp value, which will remove the need for generator efficiency entirely.