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

tile_blocksize() may fail with multi-layer rasters #87

Closed
Aariq opened this issue Aug 19, 2024 · 4 comments · Fixed by #90
Closed

tile_blocksize() may fail with multi-layer rasters #87

Aariq opened this issue Aug 19, 2024 · 4 comments · Fixed by #90
Assignees
Labels
bug Something isn't working terra related to compatibility with `terra`

Comments

@Aariq
Copy link
Collaborator

Aariq commented Aug 19, 2024

https://github.com/njtierney/geotargets/blob/ea1f9fe737ebcc64a9e6bd88f589e7554e87f17f/R/tar_terra_tiles.R#L289C5-L289C76

Not sure how I missed this, but getTileExtents() expects 1 or 2 numbers and fileBlocksize() appears to return two number per layer as a matrix. Need to add a test to confirm and then fix this.

Would the blocksize ever differ among layers of the same raster? Seems like they shouldn't, since they are required to have the same resolution and projection. If that is true, I'll just take the first row.

@Aariq Aariq added bug Something isn't working terra related to compatibility with `terra` labels Aug 19, 2024
@Aariq Aariq self-assigned this Aug 19, 2024
@njtierney
Copy link
Owner

Good catch on this!

Would the blocksize ever differ among layers of the same raster? Seems like they shouldn't, since they are required to have the same resolution and projection. If that is true, I'll just take the first row.

I'm not sure about this, but I'm wondering if @mdsumner, @anthonynorth, and @paleolimbot might have thoughts?

@Aariq
Copy link
Collaborator Author

Aariq commented Aug 21, 2024

I also noticed that if a raster source is memory and not a file, then fileBlocksize() returns 0,0 and tile_blocksize() errors uninformatively. Fortunately, I don't think that'll be an issue for us as long as tar_terra_tiles() is given a target name in the raster argument rather than creating a raster on the fly. On the other hand, it might be good to add an actually helpful error message just to be safe?

@mdsumner
Copy link

At the margins there might be a "dangle", an overlap where a whole block isn't filled by the actual extent of the raster, so there's a concept of "actual block size". Afaik that's only for the last row and/or last column, usually the right and bottom. Other variants would be a bit nasty I think, and definitely not common.

@mdsumner
Copy link

When talking about layers we're really talking bands in a 2D raster, ie multivariable or time series (kinda).

I think different block sizes there would be pathological, but actually possible given VRT descriptions. I'll have an explore to see if there's anything there to worry about 🙏

@Aariq Aariq closed this as completed in #90 Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working terra related to compatibility with `terra`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants