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

Bug in boundary length calculations #30

Closed
jeffreyhanson opened this Issue Oct 11, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@jeffreyhanson
Copy link
Contributor

jeffreyhanson commented Oct 11, 2017

There seems to be a bug in the generating shared boundary data for planning units when their vertices (or corners) do not align (ie. not located in close proximity). Note that this does not affect raster planning unit data. If your planning unit data are in a grid then this bug does not affect your results. Additionally, since this bug is related to the generation of boundary data it does not affect prioritisations where boundary data has been supplied by the user manually.

Given the example below, the boundary_matrix function will fail to detect edges between planning units 1-2, and 2-3. It will correctly identify edges between planning units 1-3 .

example

I would be interested in ideas to fix this. I suppose one strategy would be to implement a pre-processing step that adds in additional vertices to planning unit 2. What does everyone think? Should this bug be fixed prior to CRAN release?

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Oct 11, 2017

I think this bug should be fixed before the CRAN release, its a fairly big issue when working with boundary data.

Do you have ideas of how to add additional vertices? If I was working with vector data, I would probably convert the vector polygons to lines and intersect those, but I'm not sure how one would go about this when working with raster data.

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Oct 11, 2017

Yeah I agree it should be sorted out before CRAN release. The issue doesn't affect raster data because the raster data have a regular spatial structure so the pixel indices are used to generate boundary data.

@jeffreyhanson jeffreyhanson added this to the CRAN Release milestone Oct 11, 2017

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Oct 11, 2017

The amazing @mdsumner put together a brilliant example showing how the additional vertices can be inserted into polygons. I'll have a bash at incorporating this into the package tomorrow.

@ricschuster

This comment has been minimized.

Copy link
Member

ricschuster commented Oct 11, 2017

This looks awesome! Thanks very much @mdsumner

@jeffreyhanson

This comment has been minimized.

Copy link
Contributor

jeffreyhanson commented Oct 16, 2017

I had a go at implementing the solution but it doesn't seem to work on polygons with holes and seems to assume that the gPolygonize function will return polygons in the correct order. So, I've been working on updating the rcpp_boundary_data function to detect and add missing vertices before trying to generate the boundary data. I've added the motivating example above to the package's unit tests and it passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment