Skip to content
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

V(g) not the same as 1:vcount #1432

Open
maelle opened this issue Jul 17, 2024 · 5 comments · May be fixed by #1433
Open

V(g) not the same as 1:vcount #1432

maelle opened this issue Jul 17, 2024 · 5 comments · May be fixed by #1433

Comments

@maelle
Copy link
Contributor

maelle commented Jul 17, 2024

thanks @szhorvat

Relevant for

#1330
#1326 (looks ok actually)
#1427

@maelle maelle linked a pull request Jul 17, 2024 that will close this issue
@clpippel
Copy link
Contributor

clpippel commented Jul 17, 2024

@maelle,
I am afraid this is not going to work.
Because of the <weak reference> in V(g).
For example, consider:

g1 <- make_graph(c(1, 2, 3, 4))
index <- V(g1)
identical(index, V(g1))
# FALSE

# delete all vertices
g2 <- delete_vertices(g1, V(g1))
identical(V(g2), V(g2))
# FALSE

# This works for empty and non-empty sets V().
# For example.
all(V(g2) == seq_len(vcount(g2)))
# TRUE
all(index == seq_len(vcount(g1)))
# TRUE
all(index == V(g1))
#TRUE

Function identical() could be too strong.
Note thatidentical(1:4, c(1, 2, 3, 4)) returns FALSE.
Both parameters do not have the same class.

@maelle
Copy link
Contributor Author

maelle commented Jul 18, 2024

would equal work? feel free to suggest a change directly on the PR. I any case, thanks a ton!

@szhorvat
Copy link
Member

szhorvat commented Jul 18, 2024

My point about V(g) not being the same as 1:vcount was a general one. I did not have the time / chance to understand the issue you were working on in detail.

Maybe this perspective helps clarify what I meant:

distances(g, 1, V(g) will be faster than distances(g, 1, 1:vcount(g)), even though they are semantically equivalent. This is because V(g) indicates "all vertices" in a more efficient manner, and in a way that can be tested for in the C core, to help choose a better algorithm. Some algorithms are specialized for the "all vertices" case and the performance difference can be quite significant. From memory, some variants of transitivity() as well as igraph_ecc() should show a considerable difference.

@szhorvat
Copy link
Member

szhorvat commented Jul 18, 2024

Generally, I would recommend introducing a helper function to check if a vertex sequence vs includes all vertices in standard order.

I think all(vs == seq_len(vcount(g)) should work (as @clpippel says), but keep in mind that I don't know any R (!!). An optimization can/should be added to check attr(vs, 'is_all'). This is TRUE for V(g) specifically. All this assumes that vs is indeed of class igraph.vs.

@clpippel
Copy link
Contributor

Instead of identical(index, V(graph)) use all(index == V(graph)).
Straightforward and easy to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants