Add capability to read ADCP dual profile configurations #289
Conversation
|
Will fix tests when I know everything is functional |
The error says to install black[jupyter] ...ohh wait that is just a warning of sorts... I will try tomorrow. One other thing to check is that your black formatter is the latest version.
|
Hmm, just uninstalled the pip version and installed it from conda. Still no luck. |
|
@jmcvey3 I upgraded my black using pip I was able to then update using I would try the pip version since releases are faster vs conda and our tests use a pip install. If you don't care about that I pushed to
|
|
@willcoxe Is it alright with you if we use the 1 Mb file for a software test? I will try to trim it down to the minimum file size necessary to check all the code I updated. |
|
I don't own the file so I can't make that call I'm afraid. It's eventually intended to be published with a doi by the uni/research group who own it. |
|
@jmcvey3 is this PR ready for review? |
mhkit/dolfyn/io/nortek2_lib.py
Outdated
| if not dp: | ||
| # This block fixes skips that originate from before this file. | ||
| # Check first N id's and correct | ||
| delta = max(hwe[:N_id]) - hwe[:N_id] | ||
| for d, id in zip(delta, idx["ID"][:N_id]): | ||
| if d != 0: | ||
| FLAG = True | ||
| hwe[id == idx["ID"]] += d | ||
| ens[id == idx["ID"]] += d | ||
|
|
||
| if np.any(np.diff(ens) > 1) and FLAG: | ||
| idx["ens"] = np.unwrap(hwe.astype(np.int64), period=period) - hwe[0] | ||
|
|
||
| if np.any(np.diff(ens) > 1) and FLAG: | ||
| idx["ens"] = np.unwrap(hwe.astype(np.int64), period=period) - hwe[0] | ||
| return dp |
There was a problem hiding this comment.
Its not clear to me what is going on in this function. Why are we doing all these calculations to return a boolean dp that is not modified? Is this modifying the idx data structures in place so that we do not need to return those values (if so shouldn't we be more explicit about what this function is doing)?
There was a problem hiding this comment.
I can try removing this bit of code and see if that breaks tests, because I find a little more redundant code each time I need to make changes to this reader.
Unfortunately a lot of these readers modify data structures in place; if I had to guess the main reason this was done is to conserve memory for very large files (>1 Gb).
There was a problem hiding this comment.
A workaround for the in-place modification may be to add these data structures to the class (i.e. self.idx)
There was a problem hiding this comment.
Thanks for clarifying.
I think we can both agree that modifying in place and not being explicit about it is confusing and not best practice.
Long term I would like us to make a plan which cleans this up but short term let me know what you think is best which can be to leave as is and explicitly address in the future.
There was a problem hiding this comment.
Sounds good. In the interest of merging this PR, and knowing there is a lot of undocumented in-place modification in these readers, let's address it in a single PR in the future.
There was a problem hiding this comment.
Turns out that blip of code is unused.
# MHKiT v0.8.0 We're excited to announce the release of MHKiT v0.8.0, which brings a host of new features, enhancements, and bug fixes across various modules, ensuring compatibility with Python 3.10 and 3.11, and introducing full xarray support for more flexible data handling. Significant updates in the Wave and DOLfYN modules improve functionality and extend capabilities. ## Python 3.10 & 3.11 Support MHKiT now supports python 3.10 and 3.11. Support for 3.12 will follow in the next minor update. - #240 ## Wave Module ### Enhancements: **Automatic Threshold Calculation for Peaks-Over-Threshold**: We've introduced a new feature that automatically calculates the "best" threshold for identifying significant wave events. This method, originally developed by Neary, V. S., et al. in their 2020 study, has now been translated from Matlab to Python, enhancing our existing peaks-over-threshold functionality. **Wave Heights Analysis**: A new function, `wave_heights`, has been added to extract the heights of individual waves from a time series. This function uses zero up-crossing analysis to accurately measure wave heights, improving upon our previous methods which only provided the maximum value between up-crossings. **Enhanced Zero Crossing Analysis**: Building on the above, the zero crossing code previously embedded in `global_peaks` has been isolated into a helper function. This modular approach not only refines the codebase but also supports new functionalities such as calculating wave heights, zero crossing periods, and identifying crests. ### Bug Fixes: **Contour Sampling Error in Wave Contours**: A bug identified in `mhkit.wave.contours.samples_contour` has been resolved. The issue occurred when period samples defined using the maximum period resulted in values outside the interpolation range of the contour data. This was corrected by ensuring that all sampling points are within the interpolation range and adjusting the contour data selection process accordingly. - #268 - #252 - #278 ## Xarray Support MHKiT functions now fully support the use of xarray for passing and returning data. - #279 - #282 - #285 - #302 - #310 ## DOLfYN Thanks to the many user contributions and users of MHKiT the DOLFYN module include a significant number of enhancements and bug fixes. ### Enhancements: **Altimeter Support**: Enhanced the Nortek Signature Reader to add capability for reading ADCP dual profile configurations. **Data Handling Improvements**: Introduced logic to skip messy header data that can accumulate during measurements collected via Nortek software on PCs and Macs. **Instrument Noise Subtraction**: Added a function to subtract instrument noise from turbulence intensity estimation using RMS calculations, providing results that differ by approximately 1% from the existing standard deviation-based "I" property. **Improved File Handling**: Updates for RDI files to handle changing "number of cells" and variable "cell sizes," which are now bin-averaged into the largest cell size. ### Bug Fixes: **Power Spectra Calculation**: Fixed a bug where a given noise value was not being subtracted from the power spectra, and noise was inadvertently added as an input to dissipation rate calculations. **Improved Header Handling**: Allowed RDI reader to skip junk headers effectively. **Nortek Reader C Types Update**: Adjusted C types in the Nortek reader to handle below-zero water temperatures and to allow pitch and roll values to go negative. - #280 - #289 - #290 - #292 - #293 - #294 - #299 ## River & Tidal: D3D Added limits to `variable_interpolation` and added 3 array input capability to `create_points` - #271 ## Developer Experience ### Black formatting Black formatting is now enforced on all MHKiT files. This ensures consistent formatting across the MHKiT package. - #281 ### Linting & Type Hints MHKiT is in the process of enforcing pylint and adding type hints to all functions. Currently this has been achieved and is enforced in the Loads and Power modules. - #288 - #296 ### CI/CD This release introduces significant reduction in testing time for development. This is achieved by reducing the number of tests for pulls against the develop branch and only running hindcast test when changes are made to it. A bug in the hindcast CI was fixed which only ran on changes to the hindcast tests instead of the hindcast module. Additionally the wave and wind hindcast needed to be separated in 2 jobs due to the excessive time taken to run a wind cache. This created a number of follow on PRs around solidifying the logic of these job. A special case for Python 3.8, pip, and Mac OS was added to use homebrew to install NetCDF and HDF5 to get tests to pass. - #241 - #270 - #306 - #311 - #317 - #318 - #319 - #320 - #324 ### Clean Up MHKiT fixed an implementation error where functions used assert instead of built in errors for type and value checking. Additionally these PRs removed unused files, fixed typos, and created an argument which allows users to run CDIP API calls silently. - #276 - #272 - #273
Fix for #287, which is data from a dual profiling ADCP taking a large combination of burst and average measurements.