Closed Graham-EGI closed 7 months ago
@Graham-EGI thank you for submitting this report.
I looked into it and I believe the issue is here: https://github.com/MHKiT-Software/MHKiT-Python/blob/master/mhkit/wave/contours.py#L1754
As the slicing is non-inclusive of the final element (e.g. aamax).
Can you confirm if this is the same reason you found for the error and are still planning to submit a PR?
thanks again 🙏
@ssolson thanks for the quick reply! Yea that's what I thought the issue was too.
I'm still planning to submit this PR, and will do so this week. The above solution in mind still preserves the existing behavior, just allows the user to "silence" the errors if wanting to just throw the full Period range into np.arrange() or linspace().
No need to keep the issue open, unless it's helpful for me to reference in the upcoming PR!
@Graham-EGI I wanted to follow up on this issue. As far as I can tell this was never fixed. Do you have an unsubmitted PR or would it be better if I submitted one?
@ssolson - Graham no longer works with EGI. Perhaps @mbruggs can comment on this?
Resolved in #278
Describe the bug:
Sampling contours when using the full TP range to generate samples can throw errors about out of bounds.
To Reproduce:
Minimal working example: A minimal working example is a collection of source code and other data files which allow a bug or problem to be demonstrated and reproduced. The important feature of a minimal working example is that it is as small and as simple as possible, such that it is just sufficient to demonstrate the problem, but without any additional complexity or dependencies which will make resolution harder.
Expected behavior:
The
samples_contour
function will throw:ValueError: A value in x_new is above the interpolation range.
. Coming from theline at the end of the
samples_contour
function.Additional context:
Just wanting to make sure I'm not approaching the sampling wrong here. The contours in the above example are coming from OMAE 2020 model in Vericron package. If this is the correct way to go about selecting the period_samples to feed into the sampling function, I have a solution in place that I'll make a PR out of.
Here's the solution just in case: