Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Axis swap in affine_transform #12

Closed
VolkerH opened this issue Nov 1, 2018 · 6 comments
Closed

Axis swap in affine_transform #12

VolkerH opened this issue Nov 1, 2018 · 6 comments

Comments

@VolkerH
Copy link
Contributor

VolkerH commented Nov 1, 2018

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
Copy link
Contributor Author

VolkerH commented Nov 1, 2018

Also cc:ing @jni

@jni
Copy link
Collaborator

jni commented Dec 5, 2018

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

prog.run_kernel("affine",
data.shape[::-1], None,
d_im, res_g.data, mat_g.data)

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
Copy link
Owner

maweigert commented Dec 10, 2018

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
Copy link
Contributor Author

VolkerH commented Dec 10, 2018

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
Copy link
Contributor Author

VolkerH commented Dec 10, 2018

Rotations also work as expected now.

@maweigert
Copy link
Owner

Should be in the latest pypi release now 0.2.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants