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

DelayedConvert feature #1797

Merged
merged 10 commits into from
Nov 15, 2016
Merged

Conversation

lossyrob
Copy link
Member

@lossyrob lossyrob commented Nov 11, 2016

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
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
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
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
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
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
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