Skip to content

DelayedConvert feature - #1797

Merged
lossyrob merged 10 commits into
locationtech:masterfrom
lossyrob:feature/delayed-convert
Nov 15, 2016
Merged

lossyrob merged 10 commits into
locationtech:masterfrom
lossyrob:feature/delayed-convert

Conversation

@lossyrob

@lossyrob lossyrob commented Nov 11, 2016 •

Copy link
Copy Markdown
Member

This feature solves a very specific problem: Sometimes you want to map over a Tile, or combine a MulitbandTile's bands, or the like, and have the resulting Tile be of a different CellType then the original tile. For instance, when doing NDVI, if you do this on a IntConstantNoDataCellType multiband raster:

multibandTile.combineDouble(3, 4) { (r, ir) =>
  (ir - r) / (ir + r)
}

You'll get garbage values, since the ratio produces results from [-1, 1], but our result tile will be IntConstantNoDataCelltype, which will not hold the floating point values. What you have to do now is convert it to a floating point valued raster first:

multibandTile.convert(DoubleConstantNoDataCellType).combineDouble(3, 4) { (r, ir) =>
  (ir - r) / (ir + r)
}

This is awful for performance, because not only do you have to iterate over band 3 and 4 to set each value to a double value before you do the combine, but this will actually iterate over bands you don't even care about. What you can do to get around the latter issue is this:

multibandTile.subsetBands(3, 4).convert(DoubleConstantNoDataCellType).combineDouble(0, 1) { (r, ir) =>
  (ir - r) / (ir + r)
}

but that doesn't not solve the latter problem.

This solves this particular brand of problem by creating a delayedConversion method, which returns a Tile or MultibandTile that is exactly like the parent tile, except that for any method that produces a Tile or MultibandTile, that resulting tile will be an ArrayTile or ArrayMultibandTile with the target cell type. Our NDVI calculation now becomes:

multibandTile.delayedConversion(DoubleConstantNoDataCellType).combineDouble(3, 4) { (r, ir) =>
  (ir - r) / (ir + r)
}

And now we do the thing we want: iterate over band 3 and 4 only, and set the double NDVI value into a DoubleConstantNoDataCellType.

NOTE

Also contained in this PR is some ArrayMultibandTile optimizations.

Connects #1410

@lossyrob lossyrob changed the title [WIP] DelayedConvert feature DelayedConvert feature Nov 11, 2016
@lossyrob lossyrob changed the title DelayedConvert feature [WIP] DelayedConvert feature Nov 11, 2016
@lossyrob lossyrob changed the title [WIP] DelayedConvert feature DelayedConvert feature Nov 11, 2016
@lossyrob

Copy link
Copy Markdown
Member Author

@echeipesh ready for review

@lossyrob lossyrob added this to the 1.0 milestone Nov 12, 2016
newBands(b) = bandTile.map({ z => f(b, z) })
else if (targetCellType.isFloatingPoint)
newBands(b) = bandTile.mapDouble({ z => z })
else

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Useless else, this is already fixed in ArrayMultibandTile in this PR, just need to update here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They need to change cell type, so I changed these to use convert to be more explicit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, that's right.

else if (targetCellType.isFloatingPoint)
newBands(b) = bandTile.mapDouble({ z => z})
else
newBands(b) = bandTile.map({ z => z })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto above

@echeipesh

Copy link
Copy Markdown
Contributor

+1 after review

@lossyrob
lossyrob merged commit 76c207d into locationtech:master Nov 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants