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

Unify graph backends #30769

Closed
kliem opened this issue Oct 14, 2020 · 11 comments
Closed

Unify graph backends #30769

kliem opened this issue Oct 14, 2020 · 11 comments

Comments

@kliem
Copy link
Contributor

kliem commented Oct 14, 2020

This is a follow up to #28896.

We further unify the behavior of dense and sparse (dynamic) graph backends.

Adding and deleting edges now share common methods.

In particular, DenseCGraph and SparseCGraph also behave the same in the following way now:

  • adding an arc will add both directions, if the graph is undirected
  • deleting an arc will delete both directions, ...

Note that this ticket is also a step towards #28259:

Comparison:

sage: set_random_seed(0)
sage: edges = [(randint(0,1000), randint(0,1000)) for _ in range(100000)]
sage: import igraph
sage: g = igraph.Graph()
sage: g.add_vertices(1001)
sage: sleep(1)
sage: %time g.add_edges(edges)
CPU times: user 10.5 ms, sys: 0 ns, total: 10.5 ms
Wall time: 10.6 ms

Before:

sage: set_random_seed(0)
sage: edges = [(randint(0,1000), randint(0,1000)) for _ in range(100000)]
sage: D = DiGraph(multiedges=True, loops=True); D.add_vertices(range(1001))
sage: sleep(1)
sage: %time D.add_edges(edges)
CPU times: user 69 ms, sys: 38 µs, total: 69 ms
Wall time: 68.7 ms
sage: D = DiGraph(multiedges=False, loops=True, sparse=False); D.add_vertices(range(1001))
sage: sleep(1)
sage: %time D.add_edges(edges)
CPU times: user 32.3 ms, sys: 0 ns, total: 32.3 ms
Wall time: 32 ms

After:

sage: set_random_seed(0)
sage: edges = [(randint(0,1000), randint(0,1000)) for _ in range(100000)]
sage: D = DiGraph(multiedges=True, loops=True); D.add_vertices(range(1001))
sage: sleep(1)
sage: %time D.add_edges(edges)
CPU times: user 32.8 ms, sys: 38 µs, total: 32.8 ms
Wall time: 32.8 ms
sage: D = DiGraph(multiedges=False, loops=True, sparse=False); D.add_vertices(range(1001))
sage: sleep(1)
sage: %time D.add_edges(edges)
CPU times: user 14.6 ms, sys: 0 ns, total: 14.6 ms
Wall time: 14.6 ms

Depends on #30665

CC: @videlec @dimpase @dcoudert @slel

Component: graph theory

Keywords: graph backend

Author: Jonathan Kliem

Branch/Commit: 66b3dde

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/30769

@kliem kliem added this to the sage-9.2 milestone Oct 14, 2020
@kliem
Copy link
Contributor Author

kliem commented Oct 14, 2020

comment:1

Note that I waited about a second between creating the graphs and timing each time. If you hit the next time too fast, I got a slowdown in each case.

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Oct 14, 2020

comment:3

Replying to @kliem:

Note that I waited about a second between creating the graphs
and timing each time. If you hit the next %time too fast,
I got a slowdown in each case.

I added corresponding sleep(1) lines to the code blocks
in the ticket description. : )

@kliem kliem modified the milestones: sage-9.2, sage-9.3 Oct 14, 2020
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

c71a05efix doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2020

Changed commit from 1fdeca2 to c71a05e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8beb573bitset capacity is +1 to last index
63a2f42reorganization
60623c6unify arc functions
a4f51c9change some ordering
2d9154aunify adding of edges
8eba711unify delete edges
aabe5a4typo
3489fa7some small optimziations
66b3ddefix doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 14, 2020

Changed commit from c71a05e to 66b3dde

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@dcoudert
Copy link
Contributor

comment:7

For me this patch is good to go. Thank you.

@kliem
Copy link
Contributor Author

kliem commented Oct 15, 2020

comment:8

Thank you for reviewing.

@vbraun
Copy link
Member

vbraun commented Nov 1, 2020

Changed branch from u/gh-kliem/unify_graph_backends to 66b3dde

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

No branches or pull requests

4 participants