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

downscale_local_mean with non-integer factors? #2827

Closed
shoyer opened this issue Oct 6, 2017 · 10 comments · Fixed by #4201
Closed

downscale_local_mean with non-integer factors? #2827

shoyer opened this issue Oct 6, 2017 · 10 comments · Fixed by #4201
Labels
⏩ type: Enhancement Improve existing features
Milestone

Comments

@shoyer
Copy link

shoyer commented Oct 6, 2017

I'm interested in a version of skimage.transform.downscale_local_mean() which handles non-evenly dividing factors without padding the image. I want each pixel in the result to be an weighted average of overlapping pixels in the source, with weights equal to the area of overlap.

To be more explicit, this transformation is linear along equal axis. For example, from size 5 to size 2, it can be written as the matrix:

[[0.4, 0.4, 0.2, 0.0, 0.0],
 [0.0, 0.0, 0.2, 0.4, 0.4]]

This is actually what I expected skimage.transform.resize() to do, before I noticed the docstring warning about anti-aliasing.

Would this be a welcome addition to scikit-image in some form? I wrote a version that works for my own purposes, but my guess is that it's too slow for skimage, since I use a dense matrix multiply along each axis with matrix of weights.

@ahojnnes
Copy link
Member

ahojnnes commented Oct 8, 2017

Hi, thanks for reaching out. Even if your implementation is slow, this would be a useful option. I would integrate it into the existing downscale_local_mean function as another option. Does your implementation generalize to N-D?

@soupault soupault added the ⏩ type: Enhancement Improve existing features label Oct 8, 2017
@shoyer
Copy link
Author

shoyer commented Oct 8, 2017

Does your implementation generalize to N-D?

Yes, it works in N dimensions. It's written as a sequence of matrix multiplications, one along each axis.

@ahojnnes
Copy link
Member

ahojnnes commented Oct 8, 2017

Sounds great, looking forward to a pull request with the feature!

@stefanv
Copy link
Member

stefanv commented Oct 10, 2017

This has actually been requested before, so thanks for contributing!

@shoyer
Copy link
Author

shoyer commented Oct 11, 2017

Do you have any guidance on the appropriate API here?

This breaks at least two aspects of the API for downscale_local_mean:

  • factors don't need to be integers.
  • cval doesn't make sense.

Also, we can just as easily do upscaling with the local mean value as well downscaling, e.g., resizing [0, 1] to size 5 would result in [0, 0, 0.5, 1, 1].

My initial thought was to make cval=None an indicator to do resizing with fractional overlap. But then there's the separate issue of non-integer factors. An API that allows non-integer factors only if cval is None seems very awkward. I don't like the value dependent semantics. I suppose we could probably support cval when padding by a fractional factors, but the implementation is not obvious to me and that's not a feature I'm excited to write.

My tentative proposal would be a new function, perhaps resize_local_mean(image, output_shape).

@ahojnnes
Copy link
Member

ahojnnes commented Oct 11, 2017 via email

@stefanv
Copy link
Member

stefanv commented Oct 30, 2017

I think the old behavior is somewhat broken. We should definitely transition to @shoyer's implementation eventually.

Suggested route: set cval=None for the next two releases, and warn if it is not set to zero explicitly. Then, replace that function with the one proposed.

For now, let's include the function in skimage.future, with an entry in TODO.txt to move it over after the deprecation.

See also http://scikit-image.org/docs/dev/contribute.html#deprecation-cycle

@soupault soupault added this to the 0.15 milestone Jan 15, 2018
@mchamberland
Copy link

@shoyer Would you mind sharing your current implementation? I'm having trouble deriving an analytical expression for the weights of each voxel.

@shoyer
Copy link
Author

shoyer commented Jul 19, 2018

See https://gist.github.com/shoyer/c0f1ddf409667650a076c058f9a17276 for the version I wrote for my needs.

@mchamberland
Copy link

@shoyer Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants