ropensci / mapscanner

R package to print maps, draw on them, and scan them back in

https://docs.ropensci.org/mapscanner/

Other

90 stars 7 forks source link

aggregate poly faster #6

Closed mdsumner closed 5 years ago

mdsumner commented 5 years ago

this is a little faster, no sfdct

lots of purrr and piping etc, needs a cleanout

If nothing else it's gratifying to exercise these old demons, I haven't yet figured out what's core tool and what's just "technique" amongst all this

It should scale better than with sfdct, because we never expand the triangles to 4-point features - there's a bbox-cull with the pipmap - I'm sure sf does this too, but it also has every single triangle and has to push the points into the sfc list and convert to and from WKB ...

mdsumner commented 5 years ago

Does look like it scales better, but just with my hacky example


library(sf)
set.seed(6)
pts <- expand.grid(x = 1:18, y = 1:20) %>% st_as_sf(coords = c("x", "y"))
xsf <- sf::st_buffer (pts, runif(nrow(pts), 0.2, 1.5))

## THIS PR
 library(mapscanner)
> xsf <- sf::st_buffer (pts, runif(nrow(pts), 0.2, 1.5))
> system.time(out <- ms_aggregate_poly(xsf))
   user  system elapsed 
253.468   2.430 255.975 

Restarting R session...
## MASTER
> library(mapscanner)
> system.time(out <- ms_aggregate_poly(xsf))
   user  system elapsed 
398.073   2.507 400.499

mpadge commented 5 years ago

That's great - thanks! Only problem ... I've been stuffing around with that code too, including renaming the files: aggregate_poly -> aggregate_polys. I tried to fix the conflicts on my side, but it's a royal mess. Would you be able to fix on your side first? Sorry bout that ...

mdsumner commented 5 years ago

getting there

mdsumner commented 5 years ago

k, now should be fine to merge this into master

bear with me on package deps, i.e. rlang , purrr, tidyr- I've started de-piping etc. but not totally ready to do it all yet, I find that's a good way to go just pipe away until things work, then refactor (I haven't seen any performance issues in that sense anyhoo). tidyr is only there for nesting and that's only really used in spacebucket for other stuffs, but not ready to review that rn

codecov[bot] commented 5 years ago

Codecov Report

Merging #6 into master will increase coverage by 0.07%. The diff coverage is 96.25%.

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage    93.5%   93.57%   +0.07%     
==========================================
  Files           6        6              
  Lines         277      327      +50     
==========================================
+ Hits          259      306      +47     
- Misses         18       21       +3

Impacted Files	Coverage Δ
R/rectify-maps.R	`98.37% <77.77%> (-1.63%)`	:arrow_down:
R/aggregate_polys.R	`98.59% <98.59%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3f7513b...8b8c2e7. Read the comment docs.