MultibandIngest produces horizontal and vertical artifacts

[timbeatty]

The MultibandIngest tool produces unwanted visual artifacts when used with resampling method options other than nearest-neighbor.

Example screenshots:

1. nearest-neighbor (no artifacts):

2. bilinear:

3. cubic-convolution:

4. cubic-spline:

5. lanczos:

I've created a repository at tbeatty/geotrellis-ingest-example that demonstrates how to reproduce the issue with the Geotiff example from above.

[timbeatty]

I've noticed that defining cellType: int16 removes the artifacts for the bilinear resampling mode (but not cubic-spline, cubic-convolution, or lanczos).

Example:

[lossyrob]

This is perfect, thank you!

(Relevant Gitter convo)

[lossyrob]

@timbeatty just an update, I've run thought the repo and have reproduced the results (thanks again for the great testing setup). We're putting this in the queue to fix in the next week or two and are planning to put a fix into 1.2.1.

[timbeatty]

Glad to help. Thanks for the update!

[echeipesh]

@timbeatty So finally got to the bottom of this:

GeoTrellis RDD reproject, working on tiled datasets, needs to buffer each tile with pixels from its neighbor in order for reproject to have all the pixels it needs to sample at tile borders. The size of this buffer depends on the source and target projection as well the geographic location of the source tiles. GeoTrellis tries to estimate this by back-projecting the tile extents. That generally does the trick.

In this case the trick is not done. When writing Scala code there is an on option to manually specify the buffer which is not exposed through ETL configurations. For instance if I rebuild spark-etl package with the following diff:

         scheme match {
           case Left(layoutScheme: ZoomedLayoutScheme) if output.maxZoom.isDefined =>
             val LayoutLevel(zoom, layoutDefinition) = layoutScheme.levelForZoom(output.maxZoom.get)
-            zoom -> tiled.reproject(destCrs, layoutDefinition, RasterReprojectOptions(method = method, targetCellSize = Some(layoutDefinition.cellSize)))._2
+            zoom -> tiled.reproject(destCrs, layoutDefinition,
+                                   options = RasterReprojectOptions(method = method, targetCellSize = Some(layoutDefinition.cellSize)),
+                                   bufferSize = 10)._2

           case Left(layoutScheme) =>
-            tiled.reproject(destCrs, layoutScheme, method)
+            tiled.reproject(destCrs, layoutScheme, options = method, bufferSize = 10)

           case Right(layoutDefinition) =>
-            tiled.reproject(destCrs, layoutDefinition, method)
+            tiled.reproject(destCrs, layoutDefinition, options = method, bufferSize = 10)
         }
     }
   }

the artifacts in the bilinear reproject go away.

There are couple of ways to solve this problem:

1. Pad our guess for required buffer until these artifacts go away
   I'm not convinced we can do much better at guessing the buffer size, I'm sure we can pad our guess a little more and that would resolve this issue, but other projections may still experience this problem.

2. Expose bufferSize option in ETL and release 1.2.2
   In general this would break binary compatibility, but the whole point of the etl package is to be run from command line and to switch over different sources and destinations of the tiles. So potentially it doesn't make sense to follow semver here.

3. Convert usage of spark-etl to application Scala code
   Especially if switching between different kinds of backends is not a requirement for the system you're writing this could be desirable.

As an aside: we've come to the conclusion that spark-etl has a pretty big usability problem, thus our efforts to replace it with pipeline sub-project in 2.0 so I am biased against sinking effort into it. However, I'd also like to minimize user pain in whatever way makes sense.

What approach is reasonable for you here?

[timbeatty]

@echeipesh

I'm still getting to know Geotrellis, I figure it'd take me a while to tackle option 3. I'll take it on if necessary but my personal preference is for option 2 :)

[pomadchin]

Looks like a resolved thing, @timbeatty feel free to reopen this issue or to create a new one!
bufferSize option was introduced in this commit: 44b9cbb