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

Add a fix to inner_power that removes tiny wiggles #590

Closed
wants to merge 3 commits into from

Conversation

paulf81
Copy link
Collaborator

@paulf81 paulf81 commented Feb 28, 2023

This could be ready to merge

Feature or improvement description
This fix is meant to address the problem in Issue #554 (and I think along with it discussion #583). The non-flat region 3 causes odd behaviors in the optimization. I think one way we can address them is by removing very small changes in the inner_power at the time the turbine class is instantiated.

Related issue, if one exists
Issue #554
Discussion #583

Impacted areas of the software
turbine.py

Test results, if applicable
If you now re-run the example 18_check_turbine.py as in Issue #554 , the small perturbations in power are not there

@paulf81 paulf81 added the bug Something isn't working label Feb 28, 2023
dhcho347 added a commit to dhcho347/floris that referenced this pull request May 4, 2023
@rafmudaf
Copy link
Collaborator

@paulf81 I support including a correction to the power curve such as this, but I'll push back on including this within floris.simulation. In general, I'd like to keep these types of changes to user-input at the outer-most level of the code, so I suggest to incorporate this into the floris.tools instead. The simulation package should strive to remain as general purpose as possible.

That being said, it's not immediately clear where this would fit in the current floris.tools, but it might have a role in the floris.turbine_library. Overall, I think this should be considered in upcoming work on user interface improvements instead. In that regard, I suggest to reopen #554 and close this pull request.

@misi9170
Copy link
Collaborator

@paulf81 @rafmudaf An option could be to include it in turbine_utilities.py, which doesn't exist on the main or develop branches but is in the pull request #729 . However, #729 may also not have a place with planned developments of turbine.py.

@rafmudaf
Copy link
Collaborator

rafmudaf commented Nov 10, 2023

@misi9170 I like that suggestion as it starts to build a library around dealing with turbine inputs.

One workflow might be to do a preprocessing step where the power curve is smoothed and exported either as a new turbine input file or a turbine dictionary for use in the rest of the script. Something along the lines of the code below.

import floris.turbine_library as turbine_library
from floris.tools import FlorisInterface

fi = FlorisInterface(...)

# Show the wiggles
turbine_library.plot_power_curve(fi.turbine[0])

# Smooth the wiggles
corrected_turbine = turbine_library.turbine_utilities.smooth_power_curve(fi.turbine[0])
# Question: what is the type of corrected_turbine - Turbine or dict?

# Show that they're smoothed
turbine_library.plot_power_curve(corrected_turbine)

# Put this back into the simulation object
fi.reinitialize(turbine_type=corrected_turbine)

Update: I guess I just restated what @misi9170 has outlined in #729

@paulf81
Copy link
Collaborator Author

paulf81 commented Nov 10, 2023

I agree with all of this, and I think putting this correction as something to be more sensibly included in a re-worked turbine workflow, rather than forced into simulation.py will work great for me. Would you like me then to close this pull request @rafmudaf ? And re-open #554 ?

@Bartdoekemeijer
Copy link
Collaborator

To fundamentally resolve this, would it make sense to replace the power coefficient table in the turbine input file by an absolute power table? This would be a cleaner solution than adding some kind of smoothing function.

@paulf81
Copy link
Collaborator Author

paulf81 commented Nov 14, 2023

Great minds think alike! This is the proposal for 4.0, and agree this is a simpler solution

@rafmudaf rafmudaf closed this Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants