[BUGFIX] Circular upsampling across wind directions

[paulf81]

Fix upsample

(Note this is cleaned up version of #942 from a git history perspective, closing that one in favor of this one)

The upsample function in wind_data.py (both WindRose and WindTIRose) has a subtle issue. Because the current implementation assumes the same range of wind directions and wind speeds in the resampled rose as in the original, there can be a gap in the range implied by the combination of the original wind direction steps and bin size. The issue is illustrated by the code snippet:

"""Example X: Demo Wind Sample Update Issue (Temporary)

Demonstrate the issue with upsampling a wind rose

"""


import matplotlib.pyplot as plt
import numpy as np

from floris import (
    WindRose,
)

# Say that you start with a wind rose with 10-degree wind direction bins and 2 m/s wind speed bins
wind_directions = np.arange(0, 360, 10.0)
wind_speeds = np.arange(4, 20, 2.0)

# Uniform frequency
freq_table = np.ones((len(wind_directions), len(wind_speeds)))
freq_table = freq_table / np.sum(freq_table)

wind_rose = WindRose(
    wind_directions=wind_directions,
    wind_speeds=wind_speeds,
    ti_table=0.06,
    freq_table=freq_table,
)

# Upsample the wind rose to 1-degree bins
wind_rose_upsampled = wind_rose.upsample(wd_step=2.0)

# Show the original and resample wind roses
fig, axarr = plt.subplots(1, 2, figsize=(12, 5), subplot_kw={"polar": True})
wind_rose.plot(ax=axarr[0])
axarr[0].set_title("Original Wind Rose")

wind_rose_upsampled.plot(ax=axarr[1])
axarr[1].set_title("Upsampled Wind Rose")

plt.show()

Which produces:

This pull request fixes the issue by setting the new wind directions to cover the complete span of the covered directions. This change is not breaking per se, but does require changing some of the tests to match the new expected behavior. For example, a previous test was:

# Test interpolating TI along the wind direction axis
wind_directions = np.array([270, 280])
wind_speeds = np.array([6, 7])
ti_table = np.array([[0.06, 0.06], [0.07, 0.07]])
wind_rose = WindRose(wind_directions, wind_speeds, ti_table=ti_table)

wind_rose_resample = wind_rose.upsample(wd_step=5.0, ws_step=1.0, inplace=False)

np.testing.assert_allclose(wind_ti_rose_resample.wind_directions, [270, 275, 280])

But note this result only extends to 267.5, not the 265 of the original version. The new test concludes with:

np.testing.assert_allclose(wind_rose_resample.wind_directions, [267.5, 272.5, 277.5, 282.5])

With the improvements, above figure is now:

Related issue

This pull request takes into more focus work originally done within #919 but we thought should be its own pull request to make the changes clearer

Impacted areas of the software

wind_data.py

[paulf81]

@misi9170 and @ejsimley , this should've been originally a draft PR, but now it is ready for review

[misi9170]

@paulf81 thanks for the thorough PR description; that really helps make it clear what the bug was and how this works!

I've put in a few comments for you to consider, but none of them is critical I think, so I'm going ahead and approving this to ensure your other workflows can proceed. Feel free to address my comments as you see fit and merge when you're ready.

[ejsimley]

This looks like the right approach to me, and I agree, very helpful description of the problem and solution.

[paulf81]

@misi9170 and @ejsimley , sorry to do this, I had to make a few more medium-substantial changes, dealing with the wind direction range and sorting, I kept finding small bugs, and finally decided to pull out a few smaller functions to do the trickier bits so I can test a lot of possibilities directly. I think it was helpful but changed a bit the code so would appreciate a re-review, feel free to merge if happy!

[misi9170]

This looks OK to me. @ejsimley , if you're happy, I can get this merged.