mbojan / rgraph6

Encoding graphs in graph6, sparse6, and digraph6 formats
https://mbojan.github.io/rgraph6/
GNU General Public License v3.0
12 stars 3 forks source link

as_sparse6() trips on a single-edge edgelist #28

Closed mbojan closed 2 years ago

mbojan commented 2 years ago

While gnawing on #27 I stumbled upon:

g <- make_graph(~ A, B, C, D, E -- F)
as_sparse6(g)  
## [1] ":EtN"

elm <- as_edgelist(g, names=FALSE)
as_sparse6(elm, n = max(elm))
## Error in object[, 1] : incorrect number of dimensions

with

> traceback()
3: diff(c(0, object[, 1])) at as_sparse6.R#61
2: as_sparse6.matrix(elm, n = max(elm)) at as_sparse6.R#16
1: as_sparse6(elm, n = max(elm))
schochastics commented 2 years ago

That last tests in test-as_sparse6.R are wild. (i.e. "behaves correctly for edgelists with different maximums (#28)") I understand why 1 works but dont understand why 2 works and 3 fails

schochastics commented 2 years ago

Ok, I understand why 2 works and I will fix something in 3 (revert if you dont agree). The result you are checking against (":Eg^") cannot be recovered without specifying n. What can be recovered with the edge 2--3 is ":Bp", which is igraph::make_graph(~ A, B -- C). Changing it to ":Bp" makes it also compatible with test 2

schochastics commented 2 years ago

But the errors continue.

rgraph6::as_sparse6(igraph::graph.full(2))
#> [1] ":An"
rgraph6::edgelist_from_sparse6(":An")
#> Error in apply(gm[, 2:(k + 1)], 1, b2d): dim(X) must have a positive length

Might be worth it to hardcode the two possible graphs with 2 nodes, because I dont seem to find a general fix here

mbojan commented 2 years ago

Ok, I understand why 2 works and I will fix something in 3 (revert if you dont agree). The result you are checking against (":Eg^") cannot be recovered without specifying n. What can be recovered with the edge 2--3 is ":Bp", which is igraph::make_graph(~ A, B -- C). Changing it to ":Bp" makes it also compatible with test 2

Thanks for fixing that! :Bp was my intention, but I copy-pasted the S6 code from the wrong place :P

But the errors continue.

rgraph6::as_sparse6(igraph::graph.full(2))
#> [1] ":An"
rgraph6::edgelist_from_sparse6(":An")
#> Error in apply(gm[, 2:(k + 1)], 1, b2d): dim(X) must have a positive length

Might be worth it to hardcode the two possible graphs with 2 nodes, because I dont seem to find a general fix here

So now this is a bug in edgelist_from_sparse6(). I hate to hardcode stuff, but if there is no other way...

schochastics commented 2 years ago

Yes not a fan either but I couldnt find another solution. I ran all possible graphs up to 6 nodes (directed, undirected, connected, unconnected) and everything worked. So hopefully we are covering all weird cases now.