explore and replace for loops

[maelle]

if any readability/speed can be gained.

Not urgent but it might be a good way to explore different corners of the codebase.

[schochastics]

This is something that I noticed in a (admittedly niche) use case. Getting the dense adjacency matrix with a weight attribute is quite slow for larger graphs. I think this part of the code slows it down.

rigraph/R/conversion.R

Lines 206 to 236 in 7e2c5fe

	if (is_directed(graph)) {
	for (i in seq(length.out = ecount(graph))) {
	e <- ends(graph, i, names = FALSE)
	res[e[1], e[2]] <- exattr[i]
	}
	} else {
	if (type == 0) {
	## upper
	for (i in seq(length.out = ecount(graph))) {
	e <- ends(graph, i, names = FALSE)
	res[min(e), max(e)] <- exattr[i]
	}
	} else if (type == 1) {
	## lower
	for (i in seq(length.out = ecount(graph))) {
	e <- ends(graph, i, names = FALSE)
	res[max(e), min(e)] <- exattr[i]
	}
	} else if (type == 2) {
	## both
	for (i in seq(length.out = ecount(graph))) {
	e <- ends(graph, i, names = FALSE)
	res[e[1], e[2]] <- exattr[i]
	if (e[1] != e[2]) {
	res[e[2], e[1]] <- exattr[i]
	}
	}
	}
	}
	}

I was using a hacky workaround to speed it up but I dont think that this would generalize well

library(igraph)
#>
#> Attaching package: 'igraph'
#> The following objects are masked from 'package:stats':
#>
#>     decompose, spectrum
#> The following object is masked from 'package:base':
#>
#>     union
as_adj_weighted <- function(g, attr = NULL) {
    as.matrix(igraph::as_adj(g, attr = attr, type = "both", sparse = TRUE))
}


g <- sample_gnp(500, 0.2)
E(g)$weight <- runif(ecount(g))
microbenchmark::microbenchmark(
    as_adj_weighted(g, attr = "weight"),
    as_adj(g, attr = "weight",sparse = FALSE), times = 5
)
#> Unit: milliseconds
#>                                        expr        min         lq     mean
#>         as_adj_weighted(g, attr = "weight")   4.716238   4.894456 253.0044
#>  as_adj(g, attr = "weight", sparse = FALSE) 793.289684 825.885278 838.2382
#>      median        uq       max neval
#>    6.978351  34.61644 1213.8167     5
#>  850.559909 860.65557  860.8005     5

^{Created on 2024-09-19 with reprex v2.1.1}

[maelle]

Thank you @schochastics!! If you ever had time to prepare a PR for that bit of the code, we'd be super grateful!

Why do you think your solution wouldn't generalize well?

[schochastics]

hmm thinking about it I guess it might just be a matter of properly propagating the type argument. I will check this and prepare a PR eventually

[maelle]

thank you! no hurry and no pressure 😸

[schochastics]

I am going to edit this comment with other instances I find that could be sped up in a similar way:

• get.adjacency.dense when attr != NULL
• graph.incidence.dense when weighted = TRUE
• get.incidence.dense when attr != NULL

[schochastics]

There are no more instances of for loops that could be sped up significantly

[github-actions]

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.