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

higher-level ndimagefunctionality #201

Closed
martinschorb opened this issue Apr 16, 2021 · 7 comments
Closed

higher-level ndimagefunctionality #201

martinschorb opened this issue Apr 16, 2021 · 7 comments

Comments

@martinschorb
Copy link
Contributor

Hi,

I am fighting with large image arrays and found @m-albert 's amazing contribution opening up affine_transform. #159

Would it be possible to make the higher-level functions (such as rotate ) that implicitly use affine_transform available as well?

I guess that they could become part of this function family #198 .

@m-albert
Copy link
Collaborator

Hi @martinschorb, that's a great idea :) I also think it makes sense to make more interpolation functions available here!

Here are some thoughts:

I quickly went through https://github.com/scipy/scipy/blob/master/scipy/ndimage/interpolation.py and it seems only rotate relies on affine_transform. Although creating redundancy, probably for this function much of the scipy code that transforms the rotation parameters could simply be copied, followed by the call to dask_image.ndinterp.affine_transform.

Then a dask_image version of geometric_transform could probably closely mirror dask_image.ndinterp.affine_transform. Andzoom and shift might need to be handled slightly differently.

Another idea might be to have a central function that for each high-level interpolation function informs how output chunks should be mapped to input chunks, allowing for a more modular and streamlined wrapping of the scipy functions (because at the end establishing chunk correspondences is the only additional functionality dask_image should introduce).

@martinschorb Were you having anything specific in mind regarding how to best implement these functions for dask_image?

@jakirkham
Copy link
Member

In terms of reusing SciPy implementations, this is something we are discussing with upstream in issue ( scipy/scipy#10204 )

@martinschorb
Copy link
Contributor Author

@martinschorb Were you having anything specific in mind regarding how to best implement these functions for dask_image?

I was playing around with rotate and found that it accepts dask arrays as input however returns numpy arrays.
I don't think it is doing any proper parallelization, so after some digging I found that the essential interpolation is done using affine_transform. So yes a 1:1 mapping of the functions might just work in this case.
I will give this a try by copying the function code.
I guess in the long run it would be much more elegant to have some way of automatically mapping/using the supported functions from scipy as already discussed.

@martinschorb
Copy link
Contributor Author

martinschorb commented Apr 19, 2021

So, one issue I discovered when looking at rotate and also with the other high-level functions is the output parameter.

https://github.com/scipy/scipy/blob/0a8e3d226f3c6bc24451e9cd4b00543ddf673c31/scipy/ndimage/interpolation.py#L937

This calls a constructor resulting in a np.array of output size but can also fill a predefined array. However, I cannot provide a dask array beforehand, since the shape is only defined within rotate. So, to make this dask-compatible also that constructor needs to be modified.

@m-albert
Copy link
Collaborator

@gcaria I think this was just the start of the discussion, which was followed up upon in this PR by @martinschorb.

For rotating arrays you could use dask_image.ndinterp.affine_transform with a rotation matrix. If you're interested in a dask-image implementation of ndimage.rotate feel free to have a look at the PR :)

@gcaria
Copy link

gcaria commented Oct 27, 2023

Thanks @m-albert ! Sorry I had deleted my message right after posting it, because I realized there was something I had misread in the code, but here it is to make this conversation still readable:

I've just stumbled on this old issue since I too would like to rotate arrays using dask, and after a quick look at the code, I have what could be a silly question:

The output parameter used in scipy is not needed by dask_image.ndinterp.affine_transform so why is it necessary to recreate it?

@m-albert
Copy link
Collaborator

Adressed by #213.

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

4 participants