[SEDONA-449] Add two raster column support to RS_MapAlgebra #1150
Conversation
furqaankhan
changed the title
| previousRuntime.set(runtime); | ||
| } | ||
|
|
||
| // TODO is it worth to deduplicate code from 236-246 in this function |
There was a problem hiding this comment.
Oh I thought I removed it.
| Assert.assertEquals((int) expected, (int) actual); | ||
| break; | ||
| default: | ||
| Assert.assertEquals(expected, actual, 1e-3); |
There was a problem hiding this comment.
I think we already define a variable in the TestBase as the allowed precision
| val df = df1.join(df2, df1("path") === df2("path1"), "inner").select("rast0", "rast1", "path") | ||
| Seq(null, "b", "s", "i", "f", "d").foreach { pixelType => | ||
| val pixelTypeExpr = if (pixelType == null) null else s"'$pixelType'" | ||
| val dfResult = df.withColumn("rast_2", expr(s"RS_MapAlgebra(rast0, rast1, $pixelTypeExpr, 'out[0] = rast0[0] * 0.5 + rast1[0] * 0.5;', null)")) |
There was a problem hiding this comment.
Can the script have out[0]? What is the difference between out and out[0]?
There was a problem hiding this comment.
Yes, and it doesn't matter as we are return a single band raster.
| .option("pathGlobFilter", "*.tif*") | ||
| .load(resourceFolder + "raster") | ||
| .selectExpr("path", "RS_FromGeoTiff(content) as rast0") | ||
| val df2 = sparkSession.read.format("binaryFile") |
There was a problem hiding this comment.
No need to read the same dataset twice. Please use self-join: https://www.sparkcodehub.com/spark-self-join-in-spark-sql-and-dataframe
|
|
||
| // If pixelType does not match with the data type of the result image (should be double since Jiffle only supports | ||
| // double destination image), we need to convert the resultImage to the specified pixel type. | ||
| if (rasterDataType != resultImage.getSampleModel().getDataType()) { |
There was a problem hiding this comment.
What is the band data type after the map algebra? If the map algebra result is a double, and the original data type is Int, how will the final value be? Will it round the values?
There was a problem hiding this comment.
I think it will truncate them. Will have to look into it.
There was a problem hiding this comment.
What do you think about this? @Kontinuation
There was a problem hiding this comment.
convertedRaster.setSamples casts double values to the data type of the pixel type. It does not round the values.
PostGIS does something more complicated: it clamps the value according to the range of data type. It does not round the values.
There was a problem hiding this comment.
@furqaankhan can you describe the behavior of it in the doc?
| if (!cmRast0.equals(cmRast1)) { | ||
| throw new IllegalArgumentException("Color Model did not match. Provide rasters that has the same properties."); | ||
| } |
There was a problem hiding this comment.
@jiayuasu @Kontinuation Is this required to check the color models of the two input rasters?
There was a problem hiding this comment.
I don't think this check is needed. If the input rasters do have the same color model, simply let the resulting raster have a fallback color model.
) * feat: temp push of RS_MapAlgebra implementation * feat: add multi band raster test * feat: add 3 argument variant of MapAlgebra * feat: port RS_MapAlgebra to spark and add test * docs: add docs for two raster input RS_MapAlgebra * chore: remove todos * refactor: use tolerance variable in assert * refactor: use self join * refactor: remove redundant check * docs: add doc to Raster-operators * docs: add details on type casting
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-XXX] my subject.What changes were proposed in this PR?
How was this patch tested?
Did this PR include necessary documentation updates?