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

Number of areas with a specific site density not working #14

Open
Mono2 opened this issue Mar 16, 2020 · 9 comments
Open

Number of areas with a specific site density not working #14

Mono2 opened this issue Mar 16, 2020 · 9 comments
Labels
invalid This doesn't seem right

Comments

@Mono2
Copy link
Member

Mono2 commented Mar 16, 2020

The calculation of the number of areas with a specific site density does not work in the right way:

area_bug

The area marked in the picture counts as 4 areas. I think they should count as one.

@Mono2 Mono2 added the invalid This doesn't seem right label Mar 16, 2020
@RobinCGN
Copy link
Member

Maybe delete / or not count polygon part that have an area equal to the cell size (your_grid_spacing, Pixel size).

@Mono2
Copy link
Member Author

Mono2 commented Mar 17, 2020

I think if we delete or not count polygon parts that have an area equal to cell size, we will loose (maybe) too much information.
I would suggest to create a new SpatialPolygonsDataFrame (isoline_merged). Within this SPDF polygons are merged, meaning that polygons of a certain site distance enclose the hole area and not only the area of that specific site distance.
Example: The new polygon of site distance 1000 is the old polygon of 1000 + the polygon of 500.

There will be still some errors and we could delete these remaining errors.

I have implemented this function.

What do you think?

@RobinCGN
Copy link
Member

If this solves the number of areas problem. I'm fine.

But I had to change the script a bit, it corrupted the isoline_polygons spdf (only one feature left):
Zwischenablage02

Zwischenablage03

And is it correct that isoline_polygons merge contains 61 objects?
Zwischenablage02

@RobinCGN
Copy link
Member

Regarding the performance of the merge process: I have tried an alternative:

`sample <- isoline_polygons

x <- your_isoline_steps[-1]
iso_n = nrow(sample)
s <- NULL

for (i in seq(1:(length(your_isoline_steps)-1))){
s = c(s, x[i:(length(your_isoline_steps)-1)])}

st <- isoline_polygons[c(rep(seq(1,iso_n),seq(iso_n,1))),]
st@data <- cbind(st@data, x = s)
st@data
table(st$z)
st@data[st@data$x=="2500",]

isoline_polygons_merge_test <- raster::aggregate(st, by="x")
`

but your first approached performed much better:

Zwischenablage02

@Mono2
Copy link
Member Author

Mono2 commented Mar 18, 2020

I am not sure if it solves the Problem.
I added a section, that after the merging only polygons with an area larger than the cell size are counted (this is in 02_ODI_selection).
Because I am not quite sure what the best solution is we now have two entries in Isolines_stats one for the "raw" data or the old method (number_Area) and one for the merged and omitted (cell size) polygons (number_Area_merged).
All other statistical properties are based upon isoline_polygon and not upon isoline_merged.

And is it correct that isoline_polygons merge contains 61 objects?

When I run the script isoline_merged only has 60 objects as it is the case for isoline polygons

@Mono2
Copy link
Member Author

Mono2 commented Mar 19, 2020

Playing with my Rössen data, I have recognized that the grid spacing also affect this problem. If a grid with a very fine grid spacing is chosen this problem does not occur.

@Mono2 Mono2 mentioned this issue Mar 30, 2020
20 tasks
@RobinCGN
Copy link
Member

In my opinion experimental parts like "Merge polygons" should be moved to a fork and once they are ready can be merged with master. Would that be okay?

This is of course not a very urgent issue, but every time i run "01_kriging.R" it takes ages because of "Merge polygons" or I have to comment it out.

@RobinCGN
Copy link
Member

I have revised the "Merge Polygons" Function. It is a bit faster now. Number of Polygons is the same as before. Please review if you got the time.

@Mono2
Copy link
Member Author

Mono2 commented May 7, 2020

I like your new update on "Merge Polygon". Introducing the possibility of using it or not was exactly what I was thinking about! I, however, prefer to use it and I would always recommend to use it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants