[BUGFIX] Fix turbine selector in random search layout optimizer #934
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.
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, thenum_turbines
th (last) turbine was never selected.The issue stems from numpy's
randint
excluding the high value, compared to the builtinrandint
including the high value. I introduced the bug when I switched from importing the builtinrandom
to importingrandom
from numpy in 76a1465.The fix is simply to move to the high value being
num_turbines
, rather thannum_turbines - 1
. However, to avoid confusion, I've also made the usage of numpy explicit by removing the direct import ofrandom
from numpy and instead usingnp.random.randint()
.The following code demonstrates the difference between the builtin
random.randint()
andnp.random.randint()
. The firstfor
loop produces both 0 and 1s, whereas the second produces only 0s.Notes
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 thefrom numpy import random
line.