Refactor DOLfYN PSD Scripts to use Scipy.Signal.Welch

[jmcvey3]

Big refactor I'd been intending to finish for a while.

This refactor solves a DOLfYN structural problem between its binning architecture and the traditional sliding-window architecture of Welch's algorithm. Because DOLfYN was using slices of bins to compute each PSD segment, it was impossible to ensure data overlap between one bin and the next, meaning the industry-standard 50% overlap cannot be accomplished. This PR removes the bin structure from the PSD computation and ensures overlap using a "step" argument, which is also now built into the bin structure to properly average dimensions for the PSD.

In doing this, DOLfYN's custom PSD code, which was a custom implementation of Welch's algorithm without the ability to overlap FFT segments, has been replaced with scipy.signal.welch. This has one breaking change, in that NaN's are no longer tolerated in the PSD calculation.

To avoid breaking changes, the DOLfYN code will default to an overlap of 0%. This ensures array shapes will remain the same. On the other hand, since it's new, the Acoustics module will default to 50%.

The second breaking change is that the PSD code now renames the input time dimension to "time_psd". In this way, if a different overlap is used, and the PSD is saved into dataset with an averaged "time" dimension, the PSD will not follow the xarray default and go to nan.

Finally, I removed the float32 datatypes in certain functions since sometimes we do need float64 precision. Removing custom code means fft.py no longer exists, and tools/misc.py was renamed to tools.py

[akeeste]

Thanks @jmcvey3 . A couple minor comments and one question around the ADCP_example.ipynb

[jmcvey3]

Okay, @akeeste @simmsa, I believe this is completed.

[simmsa]

@jmcvey3, overall this looks good. I approve.

I have a few comments but I think we can address them after the release.

[simmsa]

Previous test used 0.01. Is it intentional that this change uses 0.001, an extra decimal?

[jmcvey3]

It's intentional to reduce the number of nans in the resulting values. Makes the test a little more robust

[simmsa]

Is the change from hamming to hann window intentional? Mostly want to make sure this change is not a typo.

[jmcvey3]

It is yes, I'm trying to make sure all of the options are covered in testing.

[simmsa]

Can we put in a check in this method and also the _csd_base method to guard against obvious out of spec user inputs to the pct_overlap parameter, i.e. 50.0.

if pct_overlap < 0 or pct_overlap > 1:
    raise ValueError(f"pct_overlap must be between 0 and 1, got {pct_overlap}.")

The specification is spelled out in the docstring, but I think we should try to prevent any out of spec values from passing into scipy through this line:

noverlap=int(pct_overlap * n_fft)

If the user does input an out of spec value it is likely that signal.welch will produce an error if noverlap > nperseg:

https://github.com/scipy/scipy/blob/54ef5423f2e4376230ec3bfda6912a07a50958e3/scipy/signal/_spectral_py.py#L913

But the error will likely be confusing. I think it would be more correct catch this on the MHKiT side.

[jmcvey3]

Yes, I should do this

[simmsa]

Is it relevant to link to the available window functions in scipy here:

https://docs.scipy.org/doc/scipy/reference/signal.windows.html#module-scipy.signal.windows

Or are we only supporting a subset of windowing functions?

[jmcvey3]

Whoa that's a lot. I just gave the most common examples, but we can link to that page

[jmcvey3]

Alright @simmsa I mentioned scipy.signal.window for window options in the docstrings, and added a check for pct_overlap. I'll merge tomorrow if there's nothing else

[simmsa]

Alright @simmsa I mentioned scipy.signal.window for window options in the docstrings, and added a check for pct_overlap. I'll merge tomorrow if there's nothing else

Thank you! Nothing else from me.