MHKiT-Python
MHKiT-Python copied to clipboard
`wave.resource.surface_elevation` does not gracefully handle input wave spectra where the zero frequency is not defined
Describe the bug:
In #250 the method
option was added to the wave.resource.surface_elevation
function. method
defaults to using the iftt
method for calculating the surface elevation. To use iftt
method the input spectrum frequency must have an zero frequency defined: https://github.com/MHKiT-Software/MHKiT-Python/blob/b8e832a1cf647a32dcca84a8d88bc878beebe765/mhkit/wave/resource.py#L276-L280 Alternatively, the sum_of_sines
method can handle frequency indexes without a zero frequency defined. If the input wave spectrum does not have a zero frequency defined should we automatically use the sum_of_sines
method instead of producing an error? Right now this option is configurable by the user, but it may make sense to automatically select the correct method
based on the input data.
To Reproduce:
import mhkit.wave as wave
import numpy as np
Hs = 1.5
Tp = 16
frequency_indexes = np.linspace(1 / 30, 1 / 2, num=32)
S = wave.resource.pierson_moskowitz_spectrum(frequency_indexes, Tp, Hs)
duration_seconds = 60 * 10
delta_time = 0.1
time_index = np.arange(0, duration_seconds, delta_time)
wave_elevation_time_series = wave.resource.surface_elevation(S, time_index)
AssertionError: ifft method must have zero frequency defined
Expected behavior:
The wave.resource.surface_elevation
function should gracefully handle an input spectrum that does have include a zero frequency defined in the input spectrum. One option would be to change the default method
to auto
and have the surface_elevation
function choose either iftt
or sum_of_sines
based on the input spectrum.
I can see that we are actively working on this code in #302. Once that is complete we should discuss if the auto
method would be an acceptable solution to this issue.
Possible Implementation:
diff --git a/mhkit/wave/resource.py b/mhkit/wave/resource.py
index b1c3a2d..3a7bbb1 100644
--- a/mhkit/wave/resource.py
+++ b/mhkit/wave/resource.py
@@ -273,11 +273,11 @@ def surface_elevation(
if not (method == "ifft" or method == "sum_of_sines"):
raise ValueError(f"Method must be 'ifft' or 'sum_of_sines'. Got: {method}")
- if method == "ifft":
+ if method == "auto" and frequency_bins is None:
if not S.index.values[0] == 0:
- raise ValueError(
- f"ifft method must have zero frequency defined. Lowest frequency is: {S.index.values[0]}"
- )
+ method = "sum_of_sines"
+ else:
+ method = "ifft"
f = pd.Series(S.index)
f.index = f
Desktop (please complete the following information):
- OS: MacOS Ventura 13.6.6
- MHKiT Version: 0.7.0
Additional context:
@ckberinger reported Issue #125 in MHKiT-MATLAB that details a use case that highlights this error. At this time the method
option is not available to the MATLAB user, (There is a working PR to add this feature).