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] Fix turbine selector in random search layout optimizer #934

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

misi9170
Copy link
Collaborator

@misi9170 misi9170 commented Jul 2, 2024

While working on #919 , @paulf81 and I found a bug that meant that, during the selection of a random turbine to move in the LayoutOptimizationRandomSearch optimization routine, the num_turbinesth (last) turbine was never selected.

The issue stems from numpy's randint excluding the high value, compared to the builtin randint including the high value. I introduced the bug when I switched from importing the builtin random to importing random from numpy in 76a1465.

The fix is simply to move to the high value being num_turbines, rather than num_turbines - 1. However, to avoid confusion, I've also made the usage of numpy explicit by removing the direct import of random from numpy and instead using np.random.randint().

The following code demonstrates the difference between the builtin random.randint() and np.random.randint(). The first for loop produces both 0 and 1s, whereas the second produces only 0s.

from random import randint
import numpy as np

print("Built in")
for _ in range(20):
    print(randint(0,1))

print("\n\nnumpy")
for _ in range(20):
    print(np.random.randint(0,1))

Notes

  • The test random_search_layout_opt_regression_test.py values were updated as this changed the regression test results.
  • Merging this into develop will likely create a small merge conflict with Add WRG file support to FLORIS #919, where this is partially fixed. I will resolve that merge conflict once this is approved and in the develop branch.
  • np.random.randint is also used in selecting initial positions in _gen_dist_based_init(). Here, the usage will mean that the (integer) maximum value will not actually be selected. However, moving 1m less than the max is essentially equivalent in this case; moreover, _gen_dist_based_init() is only used for generating initial positions that will very likely be updated by the main optimization routines. I think we don't need to change the operation of these lines, but I've still changed them to be explicit in their numpy usage after removing the from numpy import random line.

@misi9170 misi9170 added the bug Something isn't working label Jul 2, 2024
@misi9170 misi9170 requested a review from paulf81 July 2, 2024 16:22
@misi9170 misi9170 merged commit 5ea0c56 into NREL:develop Jul 2, 2024
8 checks passed
@misi9170 misi9170 deleted the bugfix/grs-turbine-select branch July 2, 2024 16:51
@misi9170 misi9170 mentioned this pull request Jul 15, 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.

2 participants