Validate and test wind direction and wind speed

[paulf81]

Validate and test wind direction and wind speed

In the development of #783, @misi9170 noticed that running this code:

fi.reinitialize(
    wind_directions=270 * np.ones_like(yaw_angles),
    wind_speeds=10.0 * np.ones_like(yaw_angles),
    turbine_type=[turbine_type]*2
)

Which has the wrong number of dimensions for wind speed and direction, produces an error related to turbulence intensity. Experimenting with the code I found that, contrary to expectations, passing in wind directions and wind speeds of different length does not produce an error, instead f_index = len(wind_directions), which is confusing. Then also, having extra dimensions in wd/ws does not immediately error, explaining the complicated errors later.

This pull request attempts to clarify the issue by:

1. Adding to the wind_direction validator a test that it is 1D
2. Add a wind_speed validator to test that it is 1D and the same length as wind_direction
   (Note TI already had a validator/test comparing lengths)

These changes are made in flow_field.py so they will catch the same errors in reinitialize as above, but also different lengths of values in an input yaml

Finally:

3. Adds test to confirm that making these errors in reinitialize raises value errors.
4. Fixing regression test that didn't conform to this standard

Impacted areas of the software

flow_field.py
tests/

[paulf81]

@rafmudaf and @misi9170 , just noticed this is causing some reg_tests to fail, this is maybe ok though, the code that fails includes lines like this:

sample_inputs_fixture.floris["flow_field"]["wind_directions"] = [270.0, 360.0]
sample_inputs_fixture.floris["flow_field"]["wind_speeds"] = [8.0]

This makes me think the validator should check for length 1, since this will get projected upwards right?

[misi9170]

@rafmudaf and @misi9170 , just noticed this is causing some reg_tests to fail, this is maybe ok though, the code that fails includes lines like this:

sample_inputs_fixture.floris["flow_field"]["wind_directions"] = [270.0, 360.0]
sample_inputs_fixture.floris["flow_field"]["wind_speeds"] = [8.0]

This makes me think the validator should check for length 1, since this will get projected upwards right?

[rafmudaf]

Let's test this on the FlowField class directly rather than indirectly through the FlorisInterface

[paulf81]

Will do!

[misi9170]

Sorry for the accidental close! Don't know how that happened. Reopened.

[rafmudaf]

For what it's worth, considering the format for array-inputs (see #794) is a scope creep. I think it would be reasonable to maintain the current design here, and develop the design for the array inputs further in the discussion thread.

The current design is that wind directions and wind speeds must be the same size, and other array-inputs can be an array of length 1 or n_findex.

[paulf81]

The current design is that wind directions and wind speeds must be the same size, and other array-inputs can be an array of length 1 or n_findex.

The problem I hit is that the reg tests don't currently follow this convention, so I would have to change them

[rafmudaf]

Based on the design we had chosen for v4, the bug is in both the regression tests and the FlowField validators. The regression tests should also be fixed in all of the rotation tests.

[paulf81]

Based on the design we had chosen for v4, the bug is in both the regression tests and the FlowField validators. The regression tests should also be fixed in all of the rotation tests.

OK I can fix both right now

[rafmudaf]

the bug is in both the regression tests and the FlowField validators.

Maybe it's more accurate to say the bug is in the FlowField validators and you've fixed those. Then, an error was raised, as it should have been, because the regression tests were incorrectly calling the API.

[paulf81]

the bug is in both the regression tests and the FlowField validators.

Maybe it's more accurate to say the bug is in the FlowField validators and you've fixed those. Then, an error was raised, as it should have been, because the regression tests were incorrectly calling the API.

agreed

[paulf81]

hi @rafmudaf I reconfigured the pull request around this discussion, is this better?

[rafmudaf]

Thanks for updating, it looks good to me