Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute surface elevation using IFFT #250

Merged
merged 4 commits into from
Jul 31, 2023

Conversation

mbruggs
Copy link
Contributor

@mbruggs mbruggs commented Jul 13, 2023

As discussed in #229 computing the surface elevation using the 'sum of sines' method is slow. Instead, we can use an IFFT which drastically increases performance. The IFFT method is used as the default when no frquency bins are provided and the frequency vector is equally spaced.

To compute the IFFT the zero frequency must be defined. The spectrum methods have been updated to avoid NaN values in the presence of the zero frequency. This means that spectrum and surface elevations functions work seamlessly together.

Previously providing a zero frequency when creating a spectrum (JONSWAP or Pierson Moskowitz) produced a NaN value. This is due the f^-5 and f^-4 terms in the calculation.

The zero frequency is a valid input and is important when calculating the surface elevation from the spectrum. We know, however, that the zero frequency should __always__ have zero amplitude as the surface elevation has a mean of zero.

This change ensures that if a zero frequency is provided, the amplitude is set to zero.
Previously the surface elevation was only computed using the 'sum of sines' method. This has been found to be prohibitively slow when computing long duration surface elevation traces.

This commit introduces the capability to compute the surface elevation using the more computationally efficient IFFT. The IFFT routine is used by default if no frequency bins are provided and the input frequency vector is equally spaced.

For example a 1hr sea state realisation at 20Hz took 11s to compute using the 'sum of sines' and 0.007s using the IFFT.

Fixes MHKiT-Software#229
@ssolson
Copy link
Contributor

ssolson commented Jul 13, 2023

@mbruggs you da man.

Looking this over I think we can get this in our next release for mid-August. I promise to get you a review before then. Currently, I am reviewing several other PRs for the next release so my review of this may be a week or so out.

Thank you so much for this contribution!

@ssolson
Copy link
Contributor

ssolson commented Jul 13, 2023

@akeeste could you review Mark's application of the IFFT? I am looking for help reviewing the application of the technique as I am not familiar with it.

@akeeste
Copy link
Contributor

akeeste commented Jul 13, 2023

@ssolson Yes definitely. @mbruggs I'll review this PR next week

@akeeste akeeste self-requested a review July 24, 2023 15:08
@akeeste akeeste self-assigned this Jul 24, 2023
Copy link
Contributor

@akeeste akeeste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mbruggs

Thanks for this great speed update. I only have a few changes to make, which I put in mbruggs#1, including

  • add doc string for frequency_bins
  • update example notebook calls to surface_elevation to use 0 frequency and the faster IFFT method

Once that is merged, I'll go ahead and bring this into MHKiT

@akeeste akeeste merged commit 3491dbb into MHKiT-Software:develop Jul 31, 2023
16 of 18 checks passed
This was referenced Aug 4, 2023
ssolson added a commit that referenced this pull request Aug 11, 2023
**MHKiT 0.7.0 Release Notes**

This release introduces exciting new features and improvements to the MHKiT package:

- **Mooring Module**: We are pleased to introduce the new mooring module. This addition primarily supports outputs from MoorDyn. Within this module, users can:
  - Import data
  - Calculate lay length
  - Visualize mooring line movements in 2D and 3D with graphical animations.
  
  Accompanying this module is an example notebook to guide users on its functionalities.

- **Dolfyn Module Revamp**: The Dolfyn module has been overhauled. Enhancements include:
  - Turbulence calculation capability
  - Performance measures for tidal power as outlined in IEC/TS 6200-200.

- **New Contributions**: A big shoutout to our community member, @mbruggs, for adding the ability to compute surface elevation using IFFT.

- **NDBC Buoy Metadata**: Users can now fetch NDBC buoy metadata directly through MHKiT.

- **Delft3D Module Update**: Stay up to date with support for the latest Delft3D NetCDF format.

**Additions**
- #235
- #232 
- #236 
- #250
- #239
- #248

**Bug Fixes**
- #226 
- #238

**Meta/Minor Changes**
- #220
- #243
- #225 
- #231
- #224

Thank you to all of the contributers who helpped with this release:
@mbruggs @Graham-EGI @castillocesar @jmcvey3 @hivanov-nrel  @browniea @cmichelenstrofer @akeeste  @maxwelllevin @rpauly18 @ssolson
@ssolson ssolson added the enhancement New feature or request label Oct 30, 2023
simmsa added a commit to simmsa/MHKiT-MATLAB that referenced this pull request Apr 22, 2024
`method` allows the user to choose between using the `ifft` or
`sum_of_sines` method for calculation of the surface elevation.

See <MHKiT-Software/MHKiT-Python#250> where this
was implemented in MHKiT-Python.
simmsa added a commit to MHKiT-Software/MHKiT-MATLAB that referenced this pull request May 10, 2024
`method` allows the user to choose between using the `ifft` or
`sum_of_sines` method for calculation of the surface elevation.

See <MHKiT-Software/MHKiT-Python#250> where this
was implemented in MHKiT-Python.
simmsa added a commit to simmsa/MHKiT-MATLAB that referenced this pull request May 13, 2024
`method` allows the user to choose between using the `ifft` or
`sum_of_sines` method for calculation of the surface elevation.

See <MHKiT-Software/MHKiT-Python#250> where this
was implemented in MHKiT-Python.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants