OpenPIV / openpiv-python-examples

Examples of https://github.com/openpiv/openpiv-python are separated from the main package
https://www.openpiv.net
MIT License
13 stars 11 forks source link

Usage of u and v for, both, displacements and velocities can be confusing. #5

Closed nepomnyi closed 1 year ago

nepomnyi commented 1 year ago

My issue.

I'm working with ensemble_correlation.ipynb example.

In block [12], we find x and y displacements in pixels using the function correlation_to_displacement(mean_correlation, nrows, ncols). But those displacements denoted as u and v.

Because they denoted as u and v, I decided that we got velocities in pix/mm there. That led me to wrong conclusions.

My suggestion.

Use different notation for displacements.

  1. Delete the current code from block [12].

  2. Introduces the following code to block [12]:

    
    # Convert correlations to displacements along horizontal and vertical axes
    xDisp, yDisp = correlation_to_displacement(mean_correlation, nrows, ncols) # the displacements in pixels

To obtain velocities from the displacements, divide the displacements by the time between the images

dt = 0.001 # s - time between the images in seconds u = xDisp / dt # pix/s - horizontal component of velocity in pixels per second y = yDisp / dt # pix/s - vertical component of velocity in pixels per second


3. Change the code in block [16] to the following:

The following is written for displacements. To obtain velocities, simply, delete by dt.

XDisp = [] YDisp = []

for i in range(corrs.shape[0]): tmpxDisp,tmpyDisp = correlation_to_displacement(corrs[i,:,:,:], nrows, ncols) XDisp.append(tmpxDisp) YDisp.append(tmpyDisp) fig, ax = subplots(figsize=(6,6)) ax.quiver(x,y,tmpxDisp,tmpyDisp,scale=200) ax.invert_yaxis() plot(tmpxDisp.mean(axis=1)*80+400,y[:,0])

XDisp = np.array(XDisp) YDisp = np.array(YDisp)

meanXDisp = np.mean(XDisp, axis=0) meanYDisp = np.mean(YDisp, axis=0)



**My comments**

As a newcomer, it was really confusing for me to see `u` and `v` used for displacements. I think, I might be not the only one. That is why I suggest using another notation for displacements and make clarifying comments. 
I understand that was explained in other examples, but I jumped right into this example without reading other examples. I think other new coming users, also, may jump right into this example. For that reason, I provided clarifying comments in my suggestion.
Even after the discussion in the google groups it took me a while to understand that 1) `u` and `v` are displacements, 2) I, simply, need to divide them by `dt` to get velocities.
I understand that `u` and `v` are used for displacements in the OpenPIV source code, but people who are just coming and unaware of that, need to see a more conventional notation for displacements in the example notebooks.
alexlib commented 1 year ago

Thanks for the suggestion @nepomnyi please fork the repo, change the notebook, and suggest the change by the pull request. Please check before that the notebook performs without errors and the other notebooks are not affected by your change.

alexlib commented 1 year ago

@nepomnyi please note that we just merged the recent set of changes to match openpiv v 0.25.0

nepomnyi commented 1 year ago

Thank you @alexlib . I was about to start working on that, when I noticed that you removed theensemble-correlation.ipynb notebook from the examples. I think I will close the current issue and will focus on providing an example with my multi-processed ensemble correlations as we discussed in our Zoom call.

alexlib commented 1 year ago

Thank you @alexlib . I was about to start working on that, when I noticed that you removed theensemble-correlation.ipynb notebook from the examples. I think I will close the current issue and will focus on providing an example with my multi-processed ensemble correlations as we discussed in our Zoom call.

that was my mistake. i recover the file and upload it back to the repo