search

maweigert / gputools

GPU accelerated image/volume processing in Python
BSD 3-Clause "New" or "Revised" License
108 stars 20 forks source link

Axis swap in affine_transform #12

Closed VolkerH closed 5 years ago

VolkerH commented 5 years ago

When testing affine_transform and comparing it to scipy.ndimage transform I get the impression that gputools swaps dimensions 0 and 2. This should become clearer when looking towards the end of this notebook:

https://github.com/VolkerH/affine_transform_debugging/blob/master/Affine_transform.ipynb

VolkerH commented 5 years ago

Also cc:ing @jni

jni commented 5 years ago

Also we uncovered #13 while trying to clarify this issue, and clarify we did: the axes of the data array are inverted here:

https://github.com/maweigert/gputools/blob/0084689494a12bc7bd2e7cf8195f78ef69af1fc1/gputools/transforms/transformations.py#L75-L77

presumably because OpenCL requires/expects (?) F-ordered data (or is it just that the kernel is implemented in this way?). However, imho the axis swapping should be invisible from the Python end, which would mean doing some transpositions of row/column reordering of the transformation matrix before handing it off to the GPU. @maweigert would you be ok with us adding a keyword option to do this, so that the matrix specification matches that in ndimage.affine_transform?

maweigert commented 5 years ago

Hi @VolkerH @jni ,

You were correct about the transform specification being different from ndimage, which I think they should be not. I fixed that in the latest develop branch to be consistent with ndimage, i.e. the matrix that gputools.transforms.affine expects is the inverse coordinate transformation (as in ndimage). I generally try to use the same convention as numpy or scipy when defining parameters, e.g. axis, so thanks for pointing that out! Could you check if the current behaviour is the indented one in your notebook? Thanks!

VolkerH commented 5 years ago

Hi,

thanks very much for these changes, very much appreciated. I tested translations and I can confirm that they do behave in the same way as in scipy.ndimage now: https://github.com/VolkerH/affine_transform_debugging/blob/master/Affine_transform_develeopment_branch_test.ipynb

If still have to test whether rotations work as expected.

If one could specify the ouput_shape of the array as in scipy.ndimage this could really be a drop-in replacement for scipy.ndimage with massive performance improvement. I see a speedup of 40x for one particular array size, even with the meagre built-in intel GPU on my laptop. Great work.

VolkerH commented 5 years ago

Rotations also work as expected now.

maweigert commented 5 years ago

Should be in the latest pypi release now 0.2.8.1