-
Notifications
You must be signed in to change notification settings - Fork 66
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
522 speed up rnet merge #550
Conversation
Heads-up @JosiahParry, I think it works fine on projected data, as long as not too far to the poles?
Please review @JosiahParry if you get a chance. Will do a benchmark... |
OK: now I realise why I thought the vectorised version failed, there's evidence of it not working.
Can you reproduce this and possibly jump in to finish this PR please @JosiahParry I'm off to bed z z |
It's now vectorised but doesn't seem to be any faster for the test example. |
The
|
Reprex to test-out speeds after checking out this branch: library(stplanr)
rnet_x = sf::read_sf("https://github.com/nptscot/npt/releases/download/v2/rnet_x_thurso.geojson")
rnet_y = sf::read_sf("https://github.com/nptscot/npt/releases/download/v2/rnet_y_thurso.geojson")
name_list = names(rnet_y)
funs = list()
# Loop through each name and assign it a function based on specific conditions
for (name in name_list) {
if (name == "geometry") {
next # Skip the current iteration
} else if (name %in% c("Gradient", "Quietness")) {
funs[[name]] = mean
} else {
funs[[name]] = sum
}
}
runtime = system.time({
rnet_merged = rnet_merge(rnet_x, rnet_y, dist = 20, segment_length = 10, funs = funs, max_angle_diff = 20)
})
nrow(rnet_x) / runtime[3] |
Bottleneck found:
Will open an new issue... |
Getting a 100x speed-up now 🎉 |
|
rnet_merge()
#522