-
Notifications
You must be signed in to change notification settings - Fork 0
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
Nov 2024 build #550
Nov 2024 build #550
Conversation
wangzhao0217
commented
Nov 4, 2024
- update parameters.json for Nov
- change orcp_city_boundary define
- checking off road path
- change filter #
- chaneg to nov
- use quantile
- 2024 Nov Build
Output on my computer:
|
Latest on this, here are some logs for end of Aberdeen and start of Edinburgh regions:
|
route_ids.csv
Outdated
@@ -1,7 +1,11 @@ | |||
nrow,plan,purpose,region,date,id | |||
111504,ebike,utility,Edinburgh and Lothians,2024-11-01,11221 | |||
111504,quietest,utility,Edinburgh and Lothians,2024-11-01,11220 | |||
<<<<<<< HEAD |
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.
Maybe tidy this up ASAP @wangzhao0217 ?
os_file_path = "inputdata/open_roads_scotland.gpkg" | ||
sf::st_geometry(os_scotland) = "geometry" | ||
|
||
system.time({ |
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.
system.time({ | |
profvis({ |
See https://cloud.r-project.org/web/packages/profvis/vignettes/profvis.html for more.
R/simplify_network.R
Outdated
|
||
# Subsetting another dataset 'rnet_y' based on the spatial relation with 'rnet_merged_all_buffer'. | ||
# It selects features from 'rnet_y' that are within the boundaries of 'rnet_merged_all_buffer'. | ||
rnet_y_subset = rnet_y[rnet_merged_all_buffer, , op = sf::st_within] | ||
rnet_y_subset = rnet_y[rnet_merged_all_projected_buffer, , op = sf::st_within] |
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.
This line of code take 95% of time
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.
Make sense.
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.
2 ideas for making this faster:
- Run the function on LA-sized (not region-sized) inputs, that will be done in Nov 2024 build #550
- Test to see if the equivalent code with
{geos}
functions is faster
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.
should be
rnet_y_subset = rnet_y[rnet_merged_all_projected_buffer, , op = sf::st_within] | |
rnet_y_subset = rnet_yp[rnet_merged_all_projected_buffer, , op = sf::st_within] |
Right?
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.
Looking good.
|
||
```{r} | ||
la_name = la_names[1] | ||
for (la_name in la_names) { |
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.
👍
la_name = la_names[1] | ||
for (la_name in la_names) { | ||
message("Processing la_name: ", la_name) | ||
la_name_lowercase = snakecase::to_snake_case(la_name) | ||
lads_la_name = lads |> filter(LAD23NM == la_name) | ||
os_scotland_la_name = os_scotland[sf::st_union(lads_la_name), , op = sf::st_intersects] | ||
# save os_scotland_la_name |
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.
Shouldn't this bit go into the #548 PR tho?
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 do, but this code just need for once
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.
👍
Currently crasshing with:
|
Bug introduced here I think: ad4f0c6 |
R/simplify_network.R
Outdated
@@ -90,13 +81,14 @@ simplify_network = function(rnet_y, parameters, region_boundary) { | |||
|
|||
# Subsetting another dataset 'rnet_y' based on the spatial relation with 'rnet_merged_all_buffer'. | |||
# It selects features from 'rnet_y' that are within the boundaries of 'rnet_merged_all_buffer'. | |||
rnet_y_subset = rnet_y[rnet_merged_all_projected_buffer, , op = sf::st_within] | |||
rnet_y_subset = sf::st_intersection(rnet_yp, rnet_merged_all) |
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.
This is 100x+ faster than what was there before, heads-up @wangzhao0217
rnet_yp_points = sf::st_point_on_surface(rnet_yp) | ||
rnet_y_points_subset = rnet_y_points[rnet_merged_all_projected_buffer, ] | ||
rnet_y_subset = rnet_yp[rnet_y_points_subset, ] | ||
rnet_yp_points_subset = rnet_yp_points[rnet_merged_all_projected_buffer, ] |
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.
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.
Correct, thanks Zhao.
* update traffic results and save CN as gpkg * add httr and set set_config(timeout(seconds = 600)) * fix error * 2 -> 600 * set to 11
Co-authored-by: Robin Lovelace <[email protected]>
Co-authored-by: Robin Lovelace <[email protected]>
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 seems that the code is not generating/uploading some of the files necessary for the web app like the rnet.pmtiles files. See here for the auto-uploaded files last time the code in this branch was run: https://github.com/nptscot/outputdata/releases/tag/v2024-11-01
@@ -1,4 +1,14 @@ | |||
nrow,plan,purpose,region,date,id | |||
93472,quietest,utility,Glasgow and Strathclyde,2024-12-01,11540 |
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.
🎉
parameters.json
Outdated
@@ -2,7 +2,7 @@ | |||
"plans": ["fastest", "balanced", "quietest", "ebike"], | |||
"min_flow": [1], | |||
"max_to_route": [1000000], | |||
"date_routing": ["2024-11-01"], | |||
"date_routing": ["2024-12-01"], |
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.
Why have you shifted it to December?
* Update boundaries, to low water mark definition * Use new lad boundaries