Round instead of ceil output shape when rotating image #3173
Conversation
Previously, we ensured that the output contained every subpixel information
from the input when rotating by ceiling the shape. This lead to inconsistent
output when rotating by multiples of 90 degrees. For example:
>>> transform.rotate(np.zeros((470, 230)), angle=90, resize=True).shape
(231, 470)
The new behavior correctly produces an output shape of (230, 470).
skimage/transform/_warps.py
Outdated
| @@ -371,7 +371,7 @@ def rotate(image, angle, resize=False, center=None, order=1, mode='constant', | |||
| maxr = corners[:, 1].max() | |||
| out_rows = maxr - minr + 1 | |||
| out_cols = maxc - minc + 1 | |||
| output_shape = np.ceil((out_rows, out_cols)) | |||
| output_shape = np.round((out_rows, out_cols)) | |||
There was a problem hiding this comment.
Not sure about this. In Python 3 this rounds to the nearest even value, which means that the function might still produce a differently shaped output if some dimensions of the input image are odd. Should this line use floor instead?
P.S. To the best of my knowledge, np.round is deprecated, and np.around is recommended instead.
There was a problem hiding this comment.
The even/odd behavior is not an issue, since it only takes effect at the 0.5 boundary, otherwise the behavior is expected. np.round/np.around is the correct choice here.
Codecov Report
@@ Coverage Diff @@
## master #3173 +/- ##
==========================================
+ Coverage 86.08% 86.09% +<.01%
==========================================
Files 338 338
Lines 27234 27237 +3
==========================================
+ Hits 23444 23449 +5
+ Misses 3790 3788 -2
Continue to review full report at Codecov.
|
|
I'm 👍 on this change. |
|
Round feels too magical to me. The fact that there is a decision boundary
on 0.5 makes the decision of whether to go up or down arbitrary (even
though we round to even, consistently, what is the defense for that case?).
Rounding up is safer, and users always have the option of specifying a size
manually. The 90 degree rotation is annoying. Is it because of floating
point imprecision that we go one pixel over?
|
|
Rounding to evens is IEEE standards correct. It eliminates bias which would otherwise be present during the rounding operation. |
|
I understand that it is "correct" for reducing bias. But it is not
necessarily correct for information preservation, or for managing expectations!
|
|
@stefanv my expectation would be to use round here. If you are only doing a single transformation, the subpixel accuracy is unlikely to matter. If you are chaining many transformations through their outputs (instead of through the transformation chaining API), then you will have much bigger problems than a missing edge pixel. The 90° rotation is a clear use case where ceil is broken. I think it should be a priority to fix it. Got any better ideas than this? |
|
My rationale was that even if we use ceil, the output image will be cut off “abpruptly” for any interpolation order but nearest neighbor interpolation. So, ceil is not any better than round in these cases, and then round is just better because it solves the rot90 problem. |
|
So imagine two images, each scaled by 0.5x. The shapes are This means that at least one data-point now falls outside of the new image. |
|
@stefanv nearest even argues the new size would be (4, 4)? But, I get your point, (9, 9) would go to (4, 4). I don't think this is a huge problem as the image should be pre-filtered for downsampling. And in the converse case of (5, 5), now you are getting data that is mostly synthesised outside of the input image. Is that worse? |
|
We are talking about rotating an image and not rescaling? If at all, the output image will be bigger than the input image if |
|
@ahojnnes I think @stefanv wanted to use the example of rescaling as a case study of what should happen. But actually for rescaling it seems we are already rounding: >>> import numpy as np
>>> from skimage import transform
>>> image = np.random.random((9, 9))
>>> transform.rescale(image, 0.5, multichannel=False).shape
(4, 4)@stefanv does this existing behaviour change your mind about what should happen with rotate? |
|
@jni In the |
Whoa! I'm not sure I like that, but ok, thanks for the info! =) |
|
Unsure of what exactly you want, but maybe if a user wants to rotate an image by a multiple of 90 degrees, they shouldn't be using transform.rotate and instead use np.rot90? https://docs.scipy.org/doc/numpy/reference/generated/numpy.rot90.html Maybe transform.rot can call np.rotate when its a multiple of 90 deg? |
|
Eh, yes, not sure I like the magic of rescale either. If we want to commit to magical fixing of coordinates, why not shrink the output coordinates by 5 * eps (or some chosen delta) and take the ceil? Still a consistent recipe, fixes the rot90 case, but does not cause sizes like 4.5 to go down to 4. |
|
@stefanv I don't think your last proposal is a good fix for the problem. This "magical threshold" that you mention will depend on the image size and might not work in many cases. How about we do a check for |
|
Hey everyone, |
|
@stefanv is the blocker here, you should go bug him in person @alexandrejaguar =P Reading the whole discussion again, I still think rounding is the correct and least-surprising choice here. |
|
It's consistent, it fixes the issue; let's put it in. |
Addresses issue #3071
Previously, we ensured that the output contained every subpixel information
from the input when rotating by ceiling the shape. This lead to inconsistent
output when rotating by multiples of 90 degrees. For example:
The new behavior correctly produces an output shape of (230, 470).