igraph / rigraph

igraph R package
https://r.igraph.org
551 stars 200 forks source link

Negative vertex IDs are handled unintuitively #699

Open szhorvat opened 1 year ago

szhorvat commented 1 year ago

In functions that take a single vertex ID as a parameter (instead of a vertex sequence), negative IDs, which should be invalid, are often handled in a strange way. Take for example shortest_paths():

> g<-make_ring(4)

This is fine:

> shortest_paths(g,1,3)$vpath
[[1]]
+ 3/4 vertices, from 9cc5663:
[1] 1 2 3

This is fine:

> shortest_paths(g,5,3)$vpath
Error in shortest_paths(g, 5, 3) : 
  At core/paths/unweighted.c:294 : cannot get shortest paths, Invalid vertex id

What is happening here?

> shortest_paths(g,-1,3)$vpath
[[1]]
+ 2/4 vertices, from 9cc5663:
[1] 2 3

It turns out that internally, shortest_paths uses as.igraph.vs() on the from parameter, and effectively indexes into the vertex ID vector. And with R's usual quirkiness, -1 means "everything except -1":

> igraph:::as.igraph.vs(g,-1)
[1] 2 3 4

Then all but the first element gets ignored. Thus, unintuitively, -1 ends up meaning 2.

szhorvat commented 1 year ago

This relates to the more general problem that the auto-generated R-to-C glue code does not properly validate scalars. If a scalar is expected, but the vector that is passed has length different from 1, then the code blindly take the first element.

When there are multiple elements, this just takes the first one, which is not a great sin, but it can still lead to confusion, as you see here. However, if there are zero elements, then we have an out-of-bounds access. This should be fixed in the interface generator.