search

MHKiT-Software / MHKiT-MATLAB

MHKiT-MATLAB 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
15 stars 23 forks source link

Add Delft3D Module #124

Closed simmsa closed 1 day ago

simmsa commented 5 months ago

Add Delft3D functionality to MHKiT-MATLAB:

Following MHKiT-Python:

simmsa commented 4 months ago

@browniea we are getting close to finishing up this PR. Could you take a look at the example livescript here and verify that everything looks OK?

Our main questions are related to the visualizations. There is not a MATLAB equivalent to tricontourf so we had to do some conversion to create grids and pass them into the contourf function. We put in some default values for the grid sizes and levels, but they differ from the MHKiT-Python example. Is the MATLAB example are these visualizations accurate?

Also, in "Comparing Face Data to Cell Data Section", would you be opposed to converting the "Replacing negative numbers close to zero with zero" into a function? I would be happy to add this function into MHKiT-Python.

browniea commented 3 months ago

@simmsa This looks good! My only comment is to change the colorbar from gradiated to discrete in the contour plots.

I'm not opposed to adding a "Replacing negative numbers close to zero with zero" function.

simmsa commented 3 months ago

@browniea, thank you for the feedback. We have updated the colorbar to use discrete steps and added python function cleanup_turbulent_kinetic_energy.

Can you take a look at the MATLAB example notebook and see if there are any other fixes we need to make.

Can also verify the logic in the cleanup_turbulent_kinetic_energy python function. Currently it takes a an array and a negative threshold value and converts all element is that array below the threshold to np.nan, and all values between the threshold and zero to zero. Does that seem correct?

simmsa commented 3 months ago

@rpauly18, any objections if we work on ADCP_Delft3D_TRTS_Example in a separate pull request? This notebook involves a combination of DOLfYN and Delft3D functionality, and to maintain the scope of this pull request, it might be better to focus solely on the completed features here. Any objections?

rpauly18 commented 3 months ago

@rpauly18, any objections if we work on ADCP_Delft3D_TRTS_Example in a separate pull request? This notebook involves a combination of DOLfYN and Delft3D functionality, and to maintain the scope of this pull request, it might be better to focus solely on the completed features here. Any objections?

That is fine with me and makes the most sense.

browniea commented 3 months ago

@browniea, thank you for the feedback. We have updated the colorbar to use discrete steps and added python function cleanup_turbulent_kinetic_energy.

Can you take a look at the MATLAB example notebook and see if there are any other fixes we need to make.

Can also verify the logic in the cleanup_turbulent_kinetic_energy python function. Currently it takes a an array and a negative threshold value and converts all element is that array below the threshold to np.nan, and all values between the threshold and zero to zero. Does that seem correct?

This all looks good. Did you add cleanup_turbulent_kinetic_energy to a Python pull request? I'm having trouble finding the code but the logic here is correct.

simmsa commented 3 months ago

@browniea, apologies for the lack of clarity. The cleanup_turbulent_kinetic_energy function is a part of this pull request. There are python functions within MHKiT-MATLAB (mhkit_python_utils) that handle some of the more complicated python sections. I added the cleanup_turbulent_kinetic_energy there.

Do you think this function would be a good addition to MHKiT-Python?

browniea commented 3 months ago

@browniea, apologies for the lack of clarity. The cleanup_turbulent_kinetic_energy function is a part of this pull request. There are python functions within MHKiT-MATLAB (mhkit_python_utils) that handle some of the more complicated python sections. I added the cleanup_turbulent_kinetic_energy there.

Do you think this function would be a good addition to MHKiT-Python?

Found it, that looks good to me. Yes, I think it would fit into MHKiT-Python as well. I'll create an issue, so I'll remember to include in my next example notebook.

simmsa commented 3 months ago

@browniea awesome, thank you for taking a look! I will work towards adding the cleanup_turbulent_kinetic_energy to MHKiT-Python. Glad it is useful.

simmsa commented 2 months ago

@rpauly18, this is ready for review.

List of changes:

Before merging we should create a new binary, but I need guidance on a version number. OK if we call this version 0.4.2? Current version is 0.4.1.

rpauly18 commented 1 month ago

In the API Doc header for each function that requires a delft_3d_py_object be passed in, please add a note directing the user to use the Delft_3d_open_netcdf function to create that object from the Delft3D file. I know it is included in the example, but someone may skip opening the netCDF file using the MHKiT function and run into issues. Similarly in the error catching in these functions, a note should be included in the test checking the Delft_3d_object type directing the user to use the Delft_3d_open_netcdf function.

simmsa commented 1 month ago

@rpauly18, 8ff57a3 improves the documentation and error messages (screenshot below) for Delft3D by directing the user to read Delft3D netCDF files using the delft_3d_open_netcdf function. 56dd091 adds a simple test to verify that an invalid input produces an error. Let me know if you have any suggestions/feedback on the error message. It seems relatively verbose, but it provides the user all of the necessary information to troubleshoot and fix the root cause. Screenshot 2024-05-29 at 1 07 02 PM