Skip to content

RDD Reproject for TileFeatures, RasterRegionReproject - #2803

Merged
echeipesh merged 6 commits into
locationtech:masterfrom
echeipesh:fix/region-reproject-mutability
Sep 15, 2018
Merged

echeipesh merged 6 commits into
locationtech:masterfrom
echeipesh:fix/region-reproject-mutability

Conversation

@echeipesh

@echeipesh echeipesh commented Sep 12, 2018 •

Copy link
Copy Markdown
Contributor

Closes #2799

Mostly this PR reworks the RasterRegionReproject to allow composition of instances such that reprojection of TileFeature RDD could be supported. Adding a unit test for this revealed another missing implicit which is added here.

The mutability remains an important optimization specifically in the RDD case where it allows using a tile as an accumulator when multiple source tiles contribute pixels during the reproject operation without allocating memory for each intermediate result. Hopefully expanded documentation is enough to steer users in the right direction and use this feature responsibly.

  • Changes RasterRegionReproject to allow creating an instance for TileFeature[T, D]
  • Uses optimized reproject logic in RasterRegionReproject[MultibandTile] instance
  • Expands scala docs to highlight and explain mutability
  • Adds Stitcher[TileFeature[T, D]] instance
  • Add errorThreshold argument from Reproject.Options to RasterRegionReproject
  • Use errorThreshold in TileRDDReproject

Checklist

  • docs/CHANGELOG.rst updated, if necessary
  • [-] docs guides update, if necessary
  • New user API has useful Scaladoc strings
  • Unit tests added for bug-fix or new feature

* @param region polygon boundry of source raster in target CRS
* @param resampleMethod cell value resample method
*/
def regionReprojectMutable(raster: Raster[T], src: CRS, dest: CRS, target: Raster[T], region: Polygon, resampleMethod: ResampleMethod): Raster[T]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange convention but needed to find a name that would not break binary compatibility.

val bands = Array.ofDim[MutableArrayTile](raster.tile.bandCount)
cfor(0)(_ < bands.length, _ + 1) { i =>
bands(i) = raster.tile.band(i).prototype(rasterExtent.cols, rasterExtent.rows).mutable
bands(i) = ArrayTile.empty(raster.tile.band(i).cellType, rasterExtent.cols, rasterExtent.rows)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change for clarity only

Raster(MultibandTile(bands), bufferRE.extent)
}

private def reprojectToBuffer(raster: Raster[MultibandTile], src: CRS, dest: CRS, buffer: Array[MutableArrayTile], rasterExtent: RasterExtent, region: Polygon, resampleMethod: ResampleMethod): Unit = {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be DRYied out with single-band case, but might as well include the most specific version with the least conditionals

@echeipesh
echeipesh requested a review from jpolchlo September 12, 2018 19:15
@echeipesh echeipesh added this to the 2.1 milestone Sep 12, 2018
@echeipesh
echeipesh force-pushed the fix/region-reproject-mutability branch from 46a25b0 to da0377f Compare September 12, 2018 21:19
@pomadchin
pomadchin force-pushed the fix/region-reproject-mutability branch 2 times, most recently from 39b42b6 to 992d523 Compare September 14, 2018 15:55
Added overload methods to keep API compatability with 2.0
@echeipesh
echeipesh force-pushed the fix/region-reproject-mutability branch from 992d523 to 5c30e09 Compare September 14, 2018 16:51
@echeipesh
echeipesh merged commit b9e0067 into locationtech:master Sep 15, 2018
@echeipesh
echeipesh deleted the fix/region-reproject-mutability branch September 15, 2018 01:13
@echeipesh

Copy link
Copy Markdown
Contributor Author

Backported to 2.1 with 2b06fdf

echeipesh added a commit that referenced this pull request Oct 2, 2018
* RasterRegionReproject works with TileFeature

* Use approximate line scan in RasterRegionReproject for performance

* Test for RDD reproject of TileFeatures

* Clarify Tile.mutable scaladoc [skip-ci]

* Pass errorThreshold to RasterRegionReproject

* Added overload methods to RasterRegionReproject keep API compatability with 2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RasterRegionReproject is broken for TileFeature (and possibly all immutable Tile types)

2 participants