igraph / rigraph

igraph R package
https://r.igraph.org
559 stars 202 forks source link

Non-numerical adjacency matrices #907

Closed szhorvat closed 10 months ago

szhorvat commented 1 year ago

At the moment, as_adjacency_matrix is mostly implemented in R. In 0.10, we have almost everything in place to rely on this C core for this. This is important because there are subtleties with counting self-loops in undirected graphs, which should work the same way across all interfaces of igraph, and more importantly: across all implementations of the adjacency matrix conversion functions. The code paths that use the C core should produce the same kinds of results as the code paths that use pure-R implementations.

There is one currently piece of functionality that is not implemented in the C core: creating adjacency matrices that are non-numerical.

Here's a small demo of this functionality with the current main branch:

> library(igraph)
> g<-make_graph(c(1,2, 2,3), directed = F)
> E(g)$foo <- c('bar', 'baz')
> as_adjacency_matrix(g, sparse=F)
     [,1] [,2] [,3]
[1,]    0    1    0
[2,]    1    0    1
[3,]    0    1    0
> as_adjacency_matrix(g, sparse=F, attr='foo')
     [,1]  [,2]  [,3] 
[1,] ""    "bar" ""   
[2,] "bar" ""    "baz"
[3,] ""    "baz" ""   
> as_adjacency_matrix(g, sparse=T, attr='foo')
Error in get.adjacency.sparse(graph, type = type, attr = attr, edges = edges,  : 
  Sparse matrices must be either numeric or logical,and the edge attribute is not
> E(g)$foo <- c(F, T)
> as_adjacency_matrix(g, sparse=T, attr='foo')
3 x 3 sparse Matrix of class "lgCMatrix"

[1,] . : .
[2,] : . |
[3,] . | .
> as_adjacency_matrix(g, sparse=F, attr='foo')
      [,1]  [,2]  [,3]
[1,] FALSE FALSE FALSE
[2,] FALSE FALSE  TRUE
[3,] FALSE  TRUE FALSE

As you can see, the dense adj mat function supports multiple different non-numeric types, while the sparse version supports numeric and logical.


@krlmlr We need to make a decision about how to handle this during the 0.10 upgrade. Please comment here so we have the decision in writing.

My suggestion:

This is for reasons of practicality. I suspect very few people use this functionality, but I'm not sure. If we have unlimited time, I'd be in favour of re-implementing it.

Finally, I'd like to note that it is somewhat pointless to use logical types as weights when building an adjacency matrix, as zeros effectively eliminate the corresponding edge. So we can't convert back to a graph from the adj mat and preserve all edges.

krlmlr commented 1 year ago

Agree to break it and see what the revdepchecks say.

krlmlr commented 11 months ago

If everything fails, we can implement the as_adjacency_matrix(g, sparse = F, attr = "foo") case by wrapping as_adjacency_matrix(g, sparse = F) and performing a lookup.

krlmlr commented 10 months ago

Seems we're good here.