Closed agila5 closed 2 years ago
Most of the time is spent running igraph::incident_edges (see here). Moreover, I don’t like the lapply approach running == for testing equality.
I found that most of the computing time in igraph::incident_edges
is spent running the following code:
if (igraph_opt("return.vs.es")) {
res <- lapply(res, function(x) create_es(graph, x + 1))
}
and from the help page of ?igraph::igraph_opt
we can read that
"return.vs.es"
: "Whether functions that return a set or sequence of vertices/edges should return formal vertex/edge sequence objects."
We don't need that feature in that part of the code (since igraph::edge.attributes()
can work with any integer input), so I (temporarily) set it to FALSE
and restore the default option immediately after running the code.
The new approach is also slightly faster than the current code (probably because it needs to process fewer nodes):
suppressPackageStartupMessages({
library(sf)
library(tidygraph)
library(sfnetworks)
})
andorra <- osmextract::oe_get("Andorra", quiet = TRUE)
andorra_sfn <- as_sfnetwork(andorra, directed = FALSE)
system.time({convert(andorra_sfn, to_spatial_smooth, .clean = TRUE)})
#> user system elapsed
#> 3.92 0.14 4.23
system.time({convert(andorra_sfn, to_spatial_smooth, .clean = TRUE, extra_fields = "highway")})
#> user system elapsed
#> 3.36 0.04 3.61
Created on 2021-05-20 by the reprex package (v2.0.0)
I'm not sure how to fix the other parts of the code (mainly the elementwise comparisons).
Hi @agila5 thanks for this! I included the edits in my updated to_spatial_smooth implementation. See the develop branch
Will fix #124 (hopefully). For the moment, I will just show the new functionality.
Load packages
Create simulated data
Plot
and create the sfn object
I think the current behaviour of to_spatial_smooth morpher (wrt to the fields) could be summarised as follows: the fields of the merged segments are all set to NA, while the other fields are kept identical to their original values.
Hence…
Now I want to modify the behaviour of to_spatial_smooth morpher according to one or more fields. In particular, now I say that a node is a pseudo-node if and only if it has one incoming and one outgoing edge (or simply two incident edges in the case of undirected networks), and all chosen attributes of these two edges are equal. Moreover, the merged edges should preserve the values of the fields specified in the extra_fields argument (since they are identical). So, for example,
We can notice that 1) there are 3 edges (instead of 2) since the morpher didn’t merge edges with different values of “type1” column; 2) the column “type1” has value “b” for the new edge since both merged edges have the value “b”.
Plot
The same code should also work with multiple fields.
If the combination of pseudo-nodes and extra_fields remove all pseudo nodes (i.e. no node satisfies the new definition), then the morpher should return the input data.
It returns an error when one or more fields do not exist in edges table:
We can apply the same idea to roxel data.
Default behaviour
Smoothing only when two incident edges have the same “type”:
The second version of the smoother has more edges (since there are fewer pseudo nodes)
but there are no NA values in the “type” column of roxel_sfn_smooth_v2
The main problem is that the new code has a “nonnegligible” impact on larger networks. For example
Most of the time is spent running
igraph::incident\_edges
(see here). Moreover, I don’t like the lapply approach running == for testing equality.Created on 2021-05-15 by the reprex package (v2.0.0)