Add Delft3D Module

[simmsa]

Add Delft3D functionality to MHKiT-MATLAB:

Following MHKiT-Python:

• [X] https://github.com/MHKiT-Software/MHKiT-Python/pull/132 - Delft3D
  • [X] get_layer_data (115b5b2307ce8e9ef1daebf280a816f8e32d6e82)
  • [X] create_points (4dd8fbe0a65d8391367e7b83db8b6572fbfb880b)
  • [X] variable_interpolation (d791fdd47813b631905c8b295c4b544551e55a12)
  • [X] get_all_data_points (3d67e6292cdac641abcd34664ebe906de31a2bb3)
  • [X] unorm (5f8e322b29c95d73a2aa07807a17b005bd5f3532)
  • [X] turbulent_intensity (930f1800733348e7b686df09f65c7aebcd914b82)
• [x] https://github.com/MHKiT-Software/MHKiT-Python/pull/168 - Delft3D
  • [X] get_all_timestamps (7d59c7887b07486b80bb7a816827e7988f8a2e16)
  • [X] convert_time (c9ad7e3a871ec135359c35de391fa03f14a60aec)
• [ ] https://github.com/MHKiT-Software/MHKiT-Python/pull/199 - Delft3D example notebook
  • [ ] Outside of current scope. Planning to add in the next PR that looks at DOLfYN
• [X] https://github.com/MHKiT-Software/MHKiT-Python/pull/248 - Delft3D
  • Python only change in get_layer_data
• [X] https://github.com/MHKiT-Software/MHKiT-Python/pull/271 - Delft3D
  • Python only changes in get_all_time, get_layer_data, create_points, get_all_data_points, and turbulent_intensity

[simmsa]

@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]

@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]

@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]

@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]

@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]

@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]

@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]

@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]

@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]

@rpauly18, this is ready for review.

List of changes:

• Add delft_3d prefixed functions that directly handle calling and returning the corresponding MHKiT-Python function.
  • Docstrings were taken directly from python with types changed
• Convert all Python output dataframes into MATLAB structures
• Convert all MATLAB incoming structures into dataframes
  • In initial versions of this PR I was keeping the python output dataframes is the matlab structure, but that seemed unnecessary. We now just convert the structures into dataframes on demand when a user passes them into a function.
• Add notebook that mirrors current MHKiT-Python functionality.
• Many fixes to get tests working on all platforms

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]

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]

@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.