diff --git a/DESCRIPTION b/DESCRIPTION index 14f59496..beb3a020 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: dodgr Title: Distances on Directed Graphs -Version: 0.4.3.038 +Version: 0.4.3.042 Authors@R: c( person("Mark", "Padgham", , "mark.padgham@email.com", role = c("aut", "cre")), person("Andreas", "Petutschnig", role = "aut"), @@ -31,7 +31,6 @@ Depends: R (>= 3.5.0) Imports: callr, - digest, fs, geodist (>= 0.1.0), magrittr, @@ -39,7 +38,8 @@ Imports: methods, osmdata, Rcpp (>= 0.12.6), - RcppParallel + RcppParallel, + secretbase Suggests: bench, dplyr, @@ -65,4 +65,4 @@ LazyData: true NeedsCompilation: yes Roxygen: list(markdown = TRUE) SystemRequirements: GNU make -Config/roxygen2/version: 8.0.0 +Config/roxygen2/version: 8.1.0 diff --git a/NAMESPACE b/NAMESPACE index f70575cc..bf4dd616 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -63,10 +63,12 @@ importFrom(graphics,plot) importFrom(magrittr,"%>%") importFrom(memoise,memoise) importFrom(methods,is) -importFrom(osmdata,add_osm_feature) -importFrom(osmdata,getbb) -importFrom(osmdata,opq) -importFrom(osmdata,osm_poly2line) -importFrom(osmdata,osmdata_sf) -importFrom(osmdata,trim_osmdata) +importFrom(osmdata, + add_osm_feature, + getbb, + opq, + osm_poly2line, + osmdata_sf, + trim_osmdata +) useDynLib(dodgr, .registration = TRUE) diff --git a/NEWS.md b/NEWS.md index 30e36db7..f64d88dd 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,8 @@ ## Minor changes +- Replace `digest` dependency with `secretbase` for all internal graph hashing/caching +- Graph hashes used for caching now also incorporate `d`, `d_weighted`, `time`, and `time_weighted` columns - Fix a few minor bugs with compound junction construction (#305, #316) - Add `pairwise` parameter to `dodgr_times()`; thanks to @leoniedu (#314) - Fix bug with categorical distances that neglected edges through compound junctions (#305) diff --git a/R/cache.R b/R/cache.R index 4234155d..5898f5f9 100644 --- a/R/cache.R +++ b/R/cache.R @@ -16,21 +16,32 @@ get_hash <- function (graph, verts = NULL, contracted = FALSE, force = FALSE) { if (contracted) { if (is.null (hash)) { gr_cols <- dodgr_graph_cols (graph) - hash <- digest::digest (list ( - graph [[gr_cols$edge_id]], names (graph), verts + hash <- secretbase::siphash13 (list ( + hash_cols (graph, gr_cols), names (graph), verts )) } } else { if (is.null (hash)) { gr_cols <- dodgr_graph_cols (graph) - hash <- digest::digest (list ( - graph [[gr_cols$edge_id]], names (graph) + hash <- secretbase::siphash13 (list ( + hash_cols (graph, gr_cols), names (graph) )) } } return (hash) } +# Columns beyond `edge_id` whose values affect `dodgr` output, and which must +# therefore also contribute to graph hashes, so that direct edits to weights +# or distances (without also changing `edge_id`) invalidate caches rather +# than silently serving stale results. See `?clear_dodgr_cache`. +hash_cols <- function (graph, gr_cols) { + nms <- c ("edge_id", "d", "d_weighted", "time", "time_weighted") + idx <- do.call (c, gr_cols [nms]) + idx <- idx [!is.na (idx)] + graph [, idx, drop = FALSE] +} + get_edge_map <- function (graph) { hashc <- get_hash (graph, contracted = TRUE) @@ -61,11 +72,11 @@ get_edge_map <- function (graph) { #' re-loaded, #' and the uncontracted version returned. #' @noRd -cache_graph <- function (graph, edge_col) { +cache_graph <- function (graph, gr_cols) { td <- fs::path_temp () - f <- function (graph, edge_col, td) { + f <- function (graph, gr_cols, td) { # the following line does not generate a coverage symbol because it is # cached, so # nocov: @@ -81,12 +92,18 @@ cache_graph <- function (graph, edge_col) { fname <- fs::path (td, paste0 ("dodgr_graph_", hash, ".Rds")) saveRDS (graph, fname) - # The hash for the contracted graph is generated from the edge IDs of - # the full graph plus default NULL vertices. Internal functions can not - # be called here, so code copied directly from `get_hash`: + # The hash for the contracted graph is generated from the edge IDs + # and weight/distance columns of the full graph plus default NULL + # vertices. Internal functions can not be called here, so code + # copied directly from `get_hash` and `hash_cols`: # hashc <- # get_hash (graph, verts = NULL, contracted = TRUE, force = TRUE) - hashc <- digest::digest (list (graph [[edge_col]], names (graph), NULL)) + hash_nms <- c ("edge_id", "d", "d_weighted", "time", "time_weighted") + hash_idx <- do.call (c, gr_cols [hash_nms]) + hash_idx <- hash_idx [!is.na (hash_idx)] + hashc <- secretbase::siphash13 (list ( + graph [, hash_idx, drop = FALSE], names (graph), NULL + )) graphc <- dodgr::dodgr_contract_graph (graph) fname_c <- fs::path (td, paste0 ("dodgr_graphc_", hashc, ".Rds")) @@ -119,7 +136,7 @@ cache_graph <- function (graph, edge_col) { } sink (file = fs::path (fs::path_temp (), "Rout.txt")) - res <- callr::r_bg (f, list (graph, edge_col, td)) + res <- callr::r_bg (f, list (graph, gr_cols, td)) sink () return (res) # R6 processx object @@ -128,11 +145,14 @@ cache_graph <- function (graph, edge_col) { #' Remove cached versions of `dodgr` graphs. #' #' This function should generally \emph{not} be needed, except if graph -#' structure has been directly modified other than through `dodgr` functions; -#' for example by modifying edge weights or distances. Graphs are cached based -#' on the vector of edge IDs, so manual changes to any other attributes will not -#' necessarily be translated into changes in `dodgr` output unless the cached -#' versions are cleared using this function. See +#' structure has been directly modified other than through `dodgr` functions. +#' Graphs are cached based on a hash of the `edge_id`, `d`, `d_weighted`, +#' `time`, and `time_weighted` columns, so manual changes to any of those will +#' be detected automatically. Manual changes to any other column or +#' attribute (for example, `from`/`to` vertex identifiers) will not be +#' reflected in the hash, and so will not necessarily be translated into +#' changes in `dodgr` output unless the cached versions are cleared using this +#' function. See #' \url{https://github.com/UrbanAnalyst/dodgr/wiki/Caching-of-streetnets-and-contracted-graphs} # nolint #' for details of caching process. #' diff --git a/R/centrality.R b/R/centrality.R index 09c4a244..0fcd67be 100644 --- a/R/centrality.R +++ b/R/centrality.R @@ -207,7 +207,7 @@ dodgr_centrality <- function (graph, if (is_dodgr_cache_on () && edges) { # re-cache graph with centrality measure: - attr (res, "px") <- cache_graph (res, gr_cols$edge_id) + attr (res, "px") <- cache_graph (res, gr_cols) } return (res) diff --git a/R/fund-cycles.R b/R/fund-cycles.R index 8850d9d7..3ac89561 100644 --- a/R/fund-cycles.R +++ b/R/fund-cycles.R @@ -100,7 +100,7 @@ dodgr_fundamental_cycles <- function (graph, # each element of res is a list, so flatten these: res <- flatten_list (res) # These hash each and remove any duplicated ones: - dig <- unlist (lapply (res, digest::digest)) + dig <- unlist (lapply (res, secretbase::siphash13)) res <- res [which (!duplicated (dig))] } diff --git a/R/graph-contraction.R b/R/graph-contraction.R index d627d29d..995e9dd6 100644 --- a/R/graph-contraction.R +++ b/R/graph-contraction.R @@ -76,7 +76,9 @@ dodgr_contract_graph <- function (graph, verts = NULL, nocache = FALSE) { graph_contracted <- dodgr_contract_graph_internal (graph, v, verts) gr_cols <- dodgr_graph_cols (graph_contracted$graph) - hashe <- digest::digest (graph_contracted$graph [[gr_cols$edge_id]]) + hashe <- secretbase::siphash13 ( + hash_cols (graph_contracted$graph, gr_cols) + ) attr (graph_contracted$graph, "hash") <- hash attr (graph_contracted$graph, "hashc") <- hashc attr (graph_contracted$graph, "hashe") <- hashe @@ -308,7 +310,7 @@ dodgr_uncontract_graph <- function (graph) { gr_cols <- dodgr_graph_cols (graph) hashe_ref <- attr (graph, "hashe") - hashe <- digest::digest (graph [[gr_cols$edge_id]]) + hashe <- secretbase::siphash13 (hash_cols (graph, gr_cols)) hash <- attr (graph, "hash") fname <- fs::path (fs::path_temp (), paste0 ("dodgr_graph_", hash, ".Rds")) diff --git a/R/graph-functions.R b/R/graph-functions.R index a389b51a..e1fd5d25 100644 --- a/R/graph-functions.R +++ b/R/graph-functions.R @@ -208,14 +208,14 @@ dodgr_vertices <- function (graph) { hash <- attr (graph, hash) # make sure rows of graph have not been changed gr_cols <- dodgr_graph_cols (graph) - hashe <- digest::digest (graph [[gr_cols$edge_id]]) + hashe <- secretbase::siphash13 (hash_cols (graph, gr_cols)) if (!identical (hashe, hash)) { hash <- NULL } if (!is.null (hash)) { hashe_ref <- attr (graph, "hashe") hashe_ref <- ifelse (is.null (hashe_ref), "", hashe_ref) - hashe <- digest::digest (graph [[gr_cols$edge_id]]) + hashe <- secretbase::siphash13 (hash_cols (graph, gr_cols)) if (hashe != hashe_ref) { hash <- hashe } diff --git a/R/weight-streetnet.R b/R/weight-streetnet.R index c3cc9b00..a2a2331c 100644 --- a/R/weight-streetnet.R +++ b/R/weight-streetnet.R @@ -408,7 +408,7 @@ weight_streetnet.sf <- function (x, hash <- get_hash (graph, contracted = FALSE, force = TRUE) attr (graph, "hash") <- hash if (is_dodgr_cache_on ()) { - attr (graph, "px") <- cache_graph (graph, gr_cols$edge_id) + attr (graph, "px") <- cache_graph (graph, gr_cols) } return (graph) @@ -772,7 +772,7 @@ weight_streetnet.sc <- function (x, get_hash (graph, contracted = FALSE, force = TRUE) if (is_dodgr_cache_on ()) { - attr (graph, "px") <- cache_graph (graph, gr_cols$edge_id) + attr (graph, "px") <- cache_graph (graph, gr_cols) } return (graph) diff --git a/codemeta.json b/codemeta.json index c562345d..74de9026 100644 --- a/codemeta.json +++ b/codemeta.json @@ -11,7 +11,7 @@ "codeRepository": "https://github.com/UrbanAnalyst/dodgr", "issueTracker": "https://github.com/UrbanAnalyst/dodgr/issues", "license": "https://spdx.org/licenses/GPL-3.0", - "version": "0.4.3.038", + "version": "0.4.3.042", "programmingLanguage": { "@type": "ComputerLanguage", "name": "R", @@ -270,18 +270,6 @@ "sameAs": "https://CRAN.R-project.org/package=callr" }, "3": { - "@type": "SoftwareApplication", - "identifier": "digest", - "name": "digest", - "provider": { - "@id": "https://cran.r-project.org", - "@type": "Organization", - "name": "Comprehensive R Archive Network (CRAN)", - "url": "https://cran.r-project.org" - }, - "sameAs": "https://CRAN.R-project.org/package=digest" - }, - "4": { "@type": "SoftwareApplication", "identifier": "fs", "name": "fs", @@ -293,7 +281,7 @@ }, "sameAs": "https://CRAN.R-project.org/package=fs" }, - "5": { + "4": { "@type": "SoftwareApplication", "identifier": "geodist", "name": "geodist", @@ -306,7 +294,7 @@ }, "sameAs": "https://CRAN.R-project.org/package=geodist" }, - "6": { + "5": { "@type": "SoftwareApplication", "identifier": "magrittr", "name": "magrittr", @@ -318,7 +306,7 @@ }, "sameAs": "https://CRAN.R-project.org/package=magrittr" }, - "7": { + "6": { "@type": "SoftwareApplication", "identifier": "memoise", "name": "memoise", @@ -330,12 +318,12 @@ }, "sameAs": "https://CRAN.R-project.org/package=memoise" }, - "8": { + "7": { "@type": "SoftwareApplication", "identifier": "methods", "name": "methods" }, - "9": { + "8": { "@type": "SoftwareApplication", "identifier": "osmdata", "name": "osmdata", @@ -347,7 +335,7 @@ }, "sameAs": "https://CRAN.R-project.org/package=osmdata" }, - "10": { + "9": { "@type": "SoftwareApplication", "identifier": "Rcpp", "name": "Rcpp", @@ -360,7 +348,7 @@ }, "sameAs": "https://CRAN.R-project.org/package=Rcpp" }, - "11": { + "10": { "@type": "SoftwareApplication", "identifier": "RcppParallel", "name": "RcppParallel", @@ -372,9 +360,21 @@ }, "sameAs": "https://CRAN.R-project.org/package=RcppParallel" }, + "11": { + "@type": "SoftwareApplication", + "identifier": "secretbase", + "name": "secretbase", + "provider": { + "@id": "https://cran.r-project.org", + "@type": "Organization", + "name": "Comprehensive R Archive Network (CRAN)", + "url": "https://cran.r-project.org" + }, + "sameAs": "https://CRAN.R-project.org/package=secretbase" + }, "SystemRequirements": "GNU make" }, - "fileSize": "36594.088KB", + "fileSize": "36598.339KB", "citation": [ { "@type": "ScholarlyArticle", diff --git a/man/clear_dodgr_cache.Rd b/man/clear_dodgr_cache.Rd index 7c434a1c..f58a7cca 100644 --- a/man/clear_dodgr_cache.Rd +++ b/man/clear_dodgr_cache.Rd @@ -11,11 +11,14 @@ Nothing; the function silently clears any cached objects } \description{ This function should generally \emph{not} be needed, except if graph -structure has been directly modified other than through \code{dodgr} functions; -for example by modifying edge weights or distances. Graphs are cached based -on the vector of edge IDs, so manual changes to any other attributes will not -necessarily be translated into changes in \code{dodgr} output unless the cached -versions are cleared using this function. See +structure has been directly modified other than through \code{dodgr} functions. +Graphs are cached based on a hash of the \code{edge_id}, \code{d}, \code{d_weighted}, +\code{time}, and \code{time_weighted} columns, so manual changes to any of those will +be detected automatically. Manual changes to any other column or +attribute (for example, \code{from}/\code{to} vertex identifiers) will not be +reflected in the hash, and so will not necessarily be translated into +changes in \code{dodgr} output unless the cached versions are cleared using this +function. See \url{https://github.com/UrbanAnalyst/dodgr/wiki/Caching-of-streetnets-and-contracted-graphs} # nolint for details of caching process. }