-
-
Notifications
You must be signed in to change notification settings - Fork 205
refactor: replace .Call() with _impl functions and improve adjacency matrix implementation
#2546
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
Changes from all commits
cf1575a
e181050
9561c7f
df951cc
b9ae7fd
8a2f0a0
5500dce
6d07b24
90ab990
593e779
fd37a96
162fe6a
7d1f778
699950e
af80285
23b177f
ad144c5
4ccab78
c47993e
6426dcc
1c6c97c
66890b7
fd14478
5348f7e
e4ced3b
017956a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -223,7 +223,6 @@ get.adjacency.dense <- function( | |
| graph, | ||
| type = c("both", "upper", "lower"), | ||
| attr = NULL, | ||
| weights = NULL, | ||
| loops = c("once", "twice", "ignore"), | ||
| names = TRUE | ||
| ) { | ||
|
|
@@ -243,20 +242,17 @@ get.adjacency.dense <- function( | |
| ) | ||
| } | ||
| loops <- igraph_match_arg(loops) | ||
| loops <- switch(loops, "ignore" = 0L, "twice" = 1L, "once" = 2L) | ||
|
|
||
| if (!is.null(weights)) { | ||
| weights <- as.numeric(weights) | ||
| # Map "ignore" to "none" for get_adjacency_impl | ||
| if (loops == "ignore") { | ||
| loops <- "none" | ||
| } | ||
|
|
||
| if (is.null(attr)) { | ||
| on.exit(.Call(Rx_igraph_finalizer)) | ||
| type <- switch(type, "upper" = 0, "lower" = 1, "both" = 2) | ||
| res <- .Call( | ||
| Rx_igraph_get_adjacency, | ||
| # FIXME: Use get_adjacency_impl() also for non-NULL attr | ||
| res <- get_adjacency_impl( | ||
| graph, | ||
| as.numeric(type), | ||
| weights, | ||
| type, | ||
| weights = numeric(), | ||
| loops | ||
|
Comment on lines
+252
to
256
|
||
| ) | ||
| } else { | ||
|
|
@@ -287,67 +283,33 @@ get.adjacency.sparse <- function( | |
|
|
||
| type <- igraph_match_arg(type) | ||
|
|
||
| vc <- vcount(graph) | ||
|
|
||
| el <- as_edgelist(graph, names = FALSE) | ||
| use.last.ij <- FALSE | ||
|
|
||
| if (!is.null(attr)) { | ||
| # Prepare weights parameter | ||
| if (is.null(attr)) { | ||
| weights <- numeric() | ||
| } else { | ||
| attr <- as.character(attr) | ||
| if (!attr %in% edge_attr_names(graph)) { | ||
| cli::cli_abort("No such edge attribute", call = call) | ||
| } | ||
| value <- edge_attr(graph, name = attr) | ||
| if (!is.numeric(value) && !is.logical(value)) { | ||
| weights <- edge_attr(graph, name = attr) | ||
| if (!is.numeric(weights) && !is.logical(weights)) { | ||
| cli::cli_abort( | ||
| "Matrices must be either numeric or logical, and the edge attribute is not", | ||
| call = call | ||
| ) | ||
| } | ||
| } else { | ||
| value <- rep(1, nrow(el)) | ||
| } | ||
|
|
||
| if (is_directed(graph)) { | ||
| res <- Matrix::sparseMatrix( | ||
| dims = c(vc, vc), | ||
| i = el[, 1], | ||
| j = el[, 2], | ||
| x = value, | ||
| use.last.ij = use.last.ij | ||
| ) | ||
| } else { | ||
| if (type == "upper") { | ||
| ## upper | ||
| res <- Matrix::sparseMatrix( | ||
| dims = c(vc, vc), | ||
| i = pmin(el[, 1], el[, 2]), | ||
| j = pmax(el[, 1], el[, 2]), | ||
| x = value, | ||
| use.last.ij = use.last.ij | ||
| ) | ||
| } else if (type == "lower") { | ||
| ## lower | ||
| res <- Matrix::sparseMatrix( | ||
| dims = c(vc, vc), | ||
| i = pmax(el[, 1], el[, 2]), | ||
| j = pmin(el[, 1], el[, 2]), | ||
| x = value, | ||
| use.last.ij = use.last.ij | ||
| ) | ||
| } else if (type == "both") { | ||
| ## both | ||
| res <- Matrix::sparseMatrix( | ||
| dims = c(vc, vc), | ||
| i = pmin(el[, 1], el[, 2]), | ||
| j = pmax(el[, 1], el[, 2]), | ||
| x = value, | ||
| symmetric = TRUE, | ||
| use.last.ij = use.last.ij | ||
| ) | ||
| res <- as(res, "generalMatrix") | ||
| } | ||
| } | ||
| # Use the library implementation | ||
| tmp <- get_adjacency_sparse_impl( | ||
| graph, | ||
| type, | ||
| weights, | ||
| loops = "once" | ||
| ) | ||
|
|
||
| # Convert to proper Matrix object | ||
| res <- igraph.i.spMatrix(tmp) | ||
|
|
||
| if (names && "name" %in% vertex_attr_names(graph)) { | ||
| colnames(res) <- rownames(res) <- V(graph)$name | ||
|
|
@@ -427,7 +389,6 @@ as_adjacency_matrix <- function( | |
| graph, | ||
| type = type, | ||
| attr = attr, | ||
| weights = NULL, | ||
| names = names, | ||
| loops = "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.
decompose_impl()expectsmodeas a character value (it usesswitch_igraph_arg(mode, ...)). In this function,modeis converted to an integer earlier and then passed here, which will error (e.g.,tolower(1L)). Pass the matched mode string ("weak"/"strong") todecompose_impl()instead and remove the numeric conversion.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.
Fixed in commit 6d07b24. Removed the numeric conversion - decompose_impl() expects the string mode parameter and handles the conversion internally.