search

MHKiT-Software / MHKiT-Python

MHKiT-Python provides the marine renewable energy (MRE) community tools for data processing, visualization, quality control, resource assessment, and device performance.
https://mhkit-software.github.io/MHKiT/
BSD 3-Clause "New" or "Revised" License
47 stars 45 forks source link

MHKiT v0.8.0 #307

Closed ssolson closed 1 month ago

ssolson commented 2 months ago

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.

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.

Xarray Support

MHKiT functions now fully support the use of xarray for passing and returning data.

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.

River & Tidal: D3D

Added limits to variable_interpolation and added 3 array input capability to create_points

Developer Experience

Black formatting

Black formatting is now enforced on all MHKiT files. This ensures consistent formatting across the MHKiT package.

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.

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.

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.

ssolson commented 2 months ago

@akeeste it looks like #302 introduced some xarray issues we did not catch in review related to the hindcast module. I need to look into why these tests were not run if we changed the hindcast module.

akeeste commented 2 months ago

Hey @ssolson I added one quick fix that I did not catch in #302. See comment here .

So I did add one minor change to the hindcast module, but the tests passed on my last commit. Apologies I should've look closer, I assumed the hindcast tests ran on that commit, but they did not. The tests is failing now because I updated a flag name from as_xarray to to_pandas to match the rest of our xarray work. I'll submit a quick fix for that into dev, see #310

ssolson commented 2 months ago

It seems like the hindcast tests are getting stuck in between the wave and wind test and then failing on a timeout. image

There is no error so I am going to need to just try a actions change in a new PR and see if that fixes it.

image

ssolson commented 2 months ago

Okay new problem. Conda on mac now fails but I believe it is just the Actions package we are using. Looking for a fix and opening a new PR.

image

image

simmsa commented 2 months ago

@ssolson, actions running macos-latest appear to be using an arm64 image of macos. This was causing trouble with MHKiT-MATLAB tests. Dropping down to macos-13 uses an image without arm in the name and fixed our issues with failing tests. We did experiment with setting the os to macos-14 but that appears to use an arm image as well.

ssolson commented 2 months ago

@ssolson, actions running macos-latest appear to be using an arm64 image of macos. This was causing trouble with MHKiT-MATLAB tests. Dropping down to macos-13 uses an image without arm in the name and fixed our issues with failing tests. We did experiment with setting the os to macos-14 but that appears to use an arm image as well.

Thanks @simmsa if this does not fix the issue I will force a specific mac os build

akeeste commented 2 months ago

Thanks @ssolson I added a couple items to the release notes above and they are good now. Do you need any support getting our tests passing?

ssolson commented 2 months ago

Thanks @ssolson I added a couple items to the release notes above and they are good now. Do you need any support getting our tests passing?

Thanks @akeeste. No mostly the issue is that the fix for one build introduces an issue for a different build. I am not stuck just juggling options to get this all working.

simmsa commented 2 months ago

@ssolson these are my opinions based on the MHKiT-MATLAB Actions.

The latest prepare-wave-hindcast-cache and prepare-wind-hindcast-cache build failures appear to be caused by Actions defaulting to python 3.12. These could probably be fixed by setting the miniconda python-version: ${{ env.PYTHON_VER }}

The conda-windows jobs failures seem to match these test failures in MHKiT-MATLAB. This seemed to be due to the capital letters in the conda environment name. We now use lowercase letters in the conda environment name and no longer have the ImportError: DLL load failed issue. I couldn't find any info about this specific error with Actions/Window/Anaconda so this may not fix this failure.

The pip-macos-latest 3.8 job failure is most likely due to the arm image of MacOS. Possible solutions are brew install netcdf or using macos-13

ssolson commented 2 months ago

@ssolson these are my opinions based on the MHKiT-MATLAB Actions.

The latest prepare-wave-hindcast-cache and prepare-wind-hindcast-cache build failures appear to be caused by Actions defaulting to python 3.12. These could probably be fixed by setting the miniconda python-version: ${{ env.PYTHON_VER }}

The conda-windows jobs failures seem to match these test failures in MHKiT-MATLAB. This seemed to be due to the capital letters in the conda environment name. We now use lowercase letters in the conda environment name and no longer have the ImportError: DLL load failed issue. I couldn't find any info about this specific error with Actions/Window/Anaconda so this may not fix this failure.

The pip-macos-latest 3.8 job failure is most likely due to the arm image of MacOS. Possible solutions are brew install netcdf or using macos-13

Thanks @simmsa the issues you are addressing here are a bit outdated and I have solved them in #318. Like I mentioned to @akeeste the solutions created new issues which I think I have now solved by using the coveralls actions. Hope to merge those changes soon.

simmsa commented 2 months ago

@ssolson, my bad! Thanks for pointing me in the right direction!

ssolson commented 1 month ago

The tests are passing there is just an edge case causing the failure on one of the 2 identical runs.

Here is how:

  1. We create a run on push to develop and PR to master which means we start 2 identical jobs when closing a PR to develop currently a. Passing push - https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8959832115 b. Failing PR - https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8959831356/attempts/1
  2. The Wind hindcast job above fails because there is another hindcast call job hitting the API at the same time
  3. Resubmitting a job currently fails I think because I had non-unique flag-names which I am addressing in #320 a. https://github.com/MHKiT-Software/MHKiT-Python/actions/runs/8959831356/attempts/2

This can be merged to master as is because the tests will now pass and no changes I make will impact the users. We could wait for #320. We could also get around this edge case by creating a v0.8.0 branch and creating a PR against master on that branch.

simmsa commented 1 month ago

@ssolson are we at the point that we should update the version number in __init__.py? Not sure when you all typically change this.

ssolson commented 1 month ago

@ssolson are we at the point that we should update the version number in __init__.py? Not sure when you all typically change this.

Thanks! Usually I like to merge to master, publish the package and then have a follow on PR after I realize my mistake.

added in 328ad1e97aa913f88e52120f2464a95aee9f5b5b

ssolson commented 1 month ago

I am going to close this PR and supersede with #321 to avoid the edge case issues above.