-
Notifications
You must be signed in to change notification settings - Fork 364
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
Viewshed #2917
Viewshed #2917
Conversation
Allow non-local illumination to compensate for the fact that the union of the pixels in the rays is not guranteed to completely cover the entire tile (especially far from the source at severe angels).
@@ -90,8 +90,12 @@ object R2Viewshed extends Serializable { | |||
* @param cols The number of columns | |||
* @param rows The number of rows | |||
*/ | |||
def generateEmptyViewshedTile(cols: Int, rows: Int) = | |||
ArrayTile.empty(IntConstantNoDataCellType, cols, rows) | |||
def generateEmptyViewshedTile(cols: Int, rows: Int, debug: Int = 0) = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like debug
should be Boolean
here, if you still want to keep this flag. (sorry that it's a bit early comment since your pr is in progress :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is an integer because it allows one to generate tiles with different "background colors" for debugging purposes. For example, setting debug = key.col + 101*key.row
in IterativeViewshed
allows one confirm in QGIS which spatial key each tile comes from. It is the visual equivalent of a printf
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesmcclain thanks for clarifying 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I only have a note about incorrect change log :)
docs/CHANGELOG.rst
Outdated
@@ -1,6 +1,14 @@ | |||
Changelog | |||
========= | |||
|
|||
3.0.1-SNAPSHOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move your update into 3.0.0-SNAPSHOT
section, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sure.
Thanks, James! |
Overview
The issue is that some fidelity is lost when visibility information is transferred between adjacent tiles. The reconstituted data, a collection of line segments associated with each tile, is not guaranteed to cover every pixel in the tile. The solution presented here is to optionally allow light to scatter perpendicularly from each ray.
There have been no changes made to the unit tests because it is unclear how to test a fix like this. Suggestions welcome.Can be tested with https://github.com/jamesmcclain/ViewshedDebug (locally-publish this branch then type./sbt "project vs" assembly
, then$SPARK_HOME/bin/spark-submit --driver-memory=8G --master='local[*]' vs-assembly-0.jar
).Artifacts
Before:
After:
Meters per Pixel
Before:
After:
Comparison:
Checklist
docs/CHANGELOG.rst
updated, if necessarydocs
guides update, if necessaryDemo
Optional. Screenshots/REPL
Notes
Optional. Ancillary topics, caveats, alternative strategies that didn't work out, anything else.
Closes #2910