Ensure interpolation values are within range when sampling contours

[ssolson]

Overview

In #218 @Graham-EGI uncovered a bug in mhkit.wave.contours.samples_contour

What he uncovered was quite simple:

• Given: Hs and Tp values
• If: you create an array from the Tp min and max to sample using samples_contour
• Then: the function could throw ValueError: A value in x_new is above the interpolation range.

Reference Example from #218

from mhkit.wave.contours import samples_contour
import numpy as np

contour = {
        "hm_contour": [ <see #218 for values> ],
        ""period_contour": [ <see #218 for values> ],
]

min_tp = min(contour["period_contour"])
max_tp = max(contour["period_contour"])

# ten samples
period_samples = np.linspace(min_tp, max_tp, 10)

# error thrown about being above the interpolation range
hm_samples = samples_contour(period_samples, np.array(contour["period_contour"]), np.array(contour["hm_contour"]))

The import part to note in the code block that period_samples is defined using max_tp.

The reason this error is thrown is because the sampling is performed by breaking the passed contour into 2 halves. The upper and lower half called w1 and w2 in samples_contour.

samples_contour chooses the upper half of the contour:

As can be seen in the second plot the upper half ("chosen segment # 2") is not inclusive of the Max Tp value (red circle on right side). This max Tp value was used as the max of the Tp sample range. Therefore the interpolation throws an error as the sample is outside the data included for interpolation.

Solution

To fix this:

1. The PR adds a check to ensure all samples are within the provided data range.
2. After choosing the upper half of the contour the PR will add 1 index value to the chosen half if the min/max sample value is outside the range of the chosen half.

[ssolson]

@cmichelenstrofer could you give me a quick sanity check on the approach I took to fix this bug?

[ssolson]

@akeeste I am 99% certain what I did is correct and was mostly just checking with Carlos just in case. If you want to give a formal review while we wait I believe this PR is ready.

[cmichelenstrofer]

Caveat: I didn't run it to make sure it works.

But the logic & implementation look good to me. If I understand correctly these are the steps in the code:

• divide the contour into two pieces
• select the top portion (higher max wave heights)
• check if the maximum period is included here, if not increase the index by one
• similar for the minimum period.

based on how the data is divided the max/minimum if missing should only be one index away.

[akeeste]

@ssolson I proposed a few changes to the logic check and how to catch both the minimum and maximum Te if either is missing. Before merging, I'd like to confirm with Graham's data tomorrow. I'll follow up tomorrow morning

[akeeste]

@ssolson I had a chance to think through this indexing more and agree that your implementation is correct, except for the minor suggestion on the logic check. After that, let's merge