Skip to content

Conversation

@browniea
Copy link
Contributor

@browniea browniea commented Sep 9, 2021

This commit would allow for importation of d3d type files.

  • get_layer _data : Get variable data from netcdf4 object at specified layer and timestep.
  • creat_points : Turns x, y, and z into a DataFrame of points to interpolate over,
    Must be provided at least two points and at most one array
  • grid_data : convert multiple variables to the same grid
  • get_all_data_points : Get data points from all layers in netcdf4 file generated from Delft3D
  • unorm : Calculates the root mean squared value given three arrays
  • turbulent_intensity: Calculated the turbulent intensity for a given data set for the specified points

The Delft3D_example notebook goes several plot options for a turbine in a flume. These options include a centerline plot, a plane contour plot and a cross section contour plot.

browniea and others added 30 commits March 2, 2021 12:04
* changing path to data following organizational update

* Reorganized headers

Co-authored-by: rpauly18 <[email protected]>
Co-authored-by: rpauly18 <[email protected]>
@ssolson
Copy link
Contributor

ssolson commented Nov 23, 2021

@browniea could you update your PR description at the top? I think you just need to delete the TODO and maybe very briefly discuss what the example notebook covers.

Copy link
Contributor

@rpauly18 rpauly18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great PR. I have added a few comments about things to be addressed and questions for consideration.

"source": [
"variable= 'ucx' \n",
"var_data_df= d3d.get_all_data_points(d3d_data, variable, time_index=1)\n",
"var_data_df\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@browniea were you trying to print out the var_data_df? If so, you will need either an explicit print statement or move the plot limits to a new cell.

@@ -0,0 +1,964 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall in the example notebook, I recommend more in line comments that explain what you are doing. The test descriptions are great, but they can get little overwhelming for novice programmers. Comments next to the code can help break it up more and explain things a bit more step by step.


def create_points(x, y, z):
'''
Turns x, y, and z into a DataFrame of points to interpolate over.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a bit more descriptive to make the use case more clear. Might just be as simple as pointing users to the example notebook for example of use.

return all_data


def unorm(x, y ,z):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssolson do you think this function would be good to have in the utils module? Its very general and could be useful elsewhere...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. @browniea please move this to the utils module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to utils @ssolson did you want me to remove it from this module?

return unorm


def turbulent_intensity(data, points='cells', time_index= -1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ssolson similarly I wonder if we would want to move this into river.resource. Eventually this type of functionality will exist for ADCP data as well. Those functions will likely end up being a bit different, but technically one could use another data source then d3d to calculate turbulence intensity. What are your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rpauly18 this function specifically takes the D3D NetCDF data structure and returns Turbulent Intensity so its a purpose-built specialized function. A general TI function in the river module would be useful but would require additional development beyond the scope of this PR IMO.

@ssolson ssolson merged commit 262e3af into MHKiT-Software:master Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants