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

WarpedVRT resampling behavior #1206

Closed
ungarj opened this issue Nov 24, 2017 · 11 comments
Closed

WarpedVRT resampling behavior #1206

ungarj opened this issue Nov 24, 2017 · 11 comments
Labels
Milestone

Comments

@ungarj
Copy link
Contributor

ungarj commented Nov 24, 2017

WarpedVRT shows unexpected behavior when using other resampling methods than Resampling.nearest.

Expected behavior and actual behavior.

Resampling.bilinear returns the same output than Resampling.nearest. However, when resampling with GDAL, the output image is smoothed.

GDAL bilinear:
gdal_bilinear

Resampling.nearest:
rio_nearest

Resampling.bilinear:
rio_bilinear

Steps to reproduce the problem.

Here is a zip file containing the example data (a CleanTOPO extract) and the script that reproduces all outputs: warpedvrt_resampling.zip

this is the code snipped that reads and resamples the window:

    with rasterio.open(input_file, "r") as src:
        with WarpedVRT(src, dst_crs=tile.crs, resampling=Resampling.bilinear) as vrt:
            bilinear = vrt.read(
                window=vrt.window(*tile.bounds()),
                out_shape=tile.shape(),
                indexes=1
            )

Not sure whether I used all of the parameters correctly. I used the example from the docs.

Operating system

Ubuntu 16.04

Rasterio version and provenance

1.0a11 installed from PyPI using pip 9.0.1, using Python 2.7

@vincentsarago
Copy link
Member

👋 @ungarj can you try to add the resampling method to the vrt.read as in https://github.com/mapbox/rio-tiler/blob/f1c3d5f86650158707fad5843d736d3e9edb082f/rio_tiler/utils.py#L134

@ungarj
Copy link
Contributor Author

ungarj commented Nov 25, 2017

That was the trick, thanks! I must have been quite blind yesterday to not try this out.

It was a bit confusing though because the WarpedVRT object also has a resampling attribute.

@ungarj ungarj closed this as completed Nov 25, 2017
@ungarj
Copy link
Contributor Author

ungarj commented Nov 25, 2017

One sidenote: adding boundless=True again leads to the nearest resampled output. But I guess, the keyword is deprecated anyway?

@ungarj
Copy link
Contributor Author

ungarj commented Dec 4, 2017

@vincentsarago resampling seems to fall back to nearest when using boundless keyword. Should I open a fresh issue or append info to this issue and reopen?

@vincentsarago
Copy link
Member

@ungarj please do

cc @sgillies

@ungarj
Copy link
Contributor Author

ungarj commented Dec 4, 2017

Ok, I did a little bit of digging:

Shouldn't here be the resampling kwarg in the read() function?
https://github.com/mapbox/rasterio/blob/44b9372c13d9696a6b355da6013e9d0a762ff47c/rasterio/_io.pyx#L362

On my local build adding it seems to work. I'm not sure though whether it's the cleanest fix because I'm not familiar enough with rasterio's internals.

Here is what I suspect: without boundless the internal _read() function is invoked with resampling and everything behaves as expected. Having boundless however triggers WarpedVRT._read() but without resampling.

It seems it is intended that the WarpedVRT.resampling attribute the VRT is initialized with should be passed on to its own internal read() function but it isn't.

Not sure how to solve that in a clean way. One option would be the solution mentioned above but this means the resampling attribute would become obsolete because it shall be passed on to read() anyway. The second option would be to overwrite the from DatasetReaderBase inherited read() and use the own resampling attirbute as default. The latter option seems to be better but I'm not sure how to properly implement this.

In any case the documentation should reflect the correct usage whether to use resampling when creating WarpedVRT and if using it with read() which resampling method is actually applied.

I can prepare a PR with the proposed changes if you want.

@ungarj
Copy link
Contributor Author

ungarj commented Dec 20, 2017

@vincentsarago done: #1238

@sgillies
Copy link
Member

@ungarj I added a long comment at #1238 and will follow up here, too.

The case of a boundless read from a WarpedVRT hasn't been well tested. We should try to avoid it and instead create instances of WarpedVRT with large enough dimensions to cover all the anticipated reads.

Rasterio's read() gets pixels from the GDAL block cache and can resample those. The resampling attribute of the WarpedVRT affects the resampling used when warping blocks of imagery that go into the cache. For better or worse, these are two different and independent operations. I will work on improving the documentation on this topic.

@ungarj
Copy link
Contributor Author

ungarj commented Dec 20, 2017

@sgillies thanks!

As mentioned in #1238, I'll try to avoid boundless read by initializing the WarpedVRT with large enough dimensions (width, height) so a boundless read won't be required.

@vincentsarago
Copy link
Member

👋 here is another demonstration of boundless=True + resampling unexpected behavior

import rasterio

src = rasterio.open('s3://remotepixel/data/image/LC82330572016015LGN00.tif')
data_expected = src.read(1, window=((0,100), (0,100)), resampling=Resampling.bilinear)

# Reading the data with boundless=True, resampling=Resampling.bilinear
data_boundless = src.read(1, window=((-100,100), (-100,100)), boundless=True, resampling=Resampling.bilinear)

# Reading the data withing the image bounds 
data_inBounds = src.read(1, window=((0,100), (0,100)), boundless=True,  resampling=Resampling.bilinear)

data_boundless != data_expected
data_inBounds != data_expected
data_inBounds == data_boundless

https://gist.github.com/vincentsarago/a58c088c7f7de6fd0806a2ba0106e147

cc @sgillies

@sgillies sgillies added the bug label May 25, 2018
@sgillies sgillies added this to To do in Rasterio 1.0.0 via automation May 25, 2018
@sgillies sgillies added this to the 1.0 milestone May 25, 2018
@sgillies sgillies removed this from To do in Rasterio 1.0.0 Jun 4, 2018
@sgillies sgillies modified the milestones: 1.0, post 1.0 Jun 4, 2018
@sgillies
Copy link
Member

This one has been closed in 1.0b4.

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

No branches or pull requests

3 participants