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 support for multiple turbine types in calculate_horizontal_plane_with_turbines() #781

Merged
merged 4 commits into from Feb 7, 2024

Conversation

scottryn
Copy link
Contributor

Add support for multiple turbine types in calculate_horizontal_plane_with_turbines()

Function now copies turbine type list from input FlorisInterface and appends a nrel_5MW to ensure correct number of turbines in fi.reinitialize() calls.

Related issue

No related open issues.

Impacted areas of the software

floris.tools.visualization.py

Additional supporting information

Small modification to prevent turbine_type must have the same number of entries as layout_x/layout_y or have a single turbine_type value error when generating a CutPlane using calculate_horizontal_plane_with_turbines() with multiple turbine definitions. Previously the test turbine was left undefined leading to a potential mismatch between the layout arguments and number of turbines if turbine_type was specified in earlier reinitializations.

Test results, if applicable

Add support for multiple turbine types by modifying copy of input turbine type list.
@misi9170 misi9170 added the bug Something isn't working label Jan 18, 2024
@rafmudaf rafmudaf self-assigned this Jan 25, 2024
@rafmudaf
Copy link
Collaborator

I've assigned myself to handle this pull request, but @paulf81 since you wrote this function could you review it first?

Copy link
Collaborator

@paulf81 paulf81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, reviewed the code changes and this is a good solution to the issue

@paulf81 paulf81 self-requested a review January 25, 2024 18:16
@paulf81
Copy link
Collaborator

paulf81 commented Jan 25, 2024

Actually hold on, let me debug the example

@paulf81
Copy link
Collaborator

paulf81 commented Jan 25, 2024

@rafmudaf I pushed a small change that I think fixes the issue that was causing example 02 to fail. In a nutshell, the turbine type array in that case is length 1, which farm.py knows to expand to n_turbines using:
_turbine_types *= self.n_turbines (line 201)

I added a similar catch/logic here

Copy link
Collaborator

@paulf81 paulf81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests/examples passing now so approving

@rafmudaf rafmudaf merged commit a88a154 into NREL:develop Feb 7, 2024
8 checks passed
@misi9170 misi9170 mentioned this pull request Apr 5, 2024
4 tasks
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.

None yet

4 participants