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

Improve Depth First Search in c_graph.pyx #31129

Closed
kliem opened this issue Dec 29, 2020 · 12 comments
Closed

Improve Depth First Search in c_graph.pyx #31129

kliem opened this issue Dec 29, 2020 · 12 comments

Comments

@kliem
Copy link
Contributor

kliem commented Dec 29, 2020

This ticket simply applies the improvements of #31117 to depth first search as well.

Before:

sage: def comp(): 
....:     for n in [5, 10, 50, 100, 500, 1000]: 
....:         G = graphs.Grid2dGraph(n, n)  
....:         print(G)  
....:         %timeit _ = list(G.depth_first_search(start=(0, 0))) 
....:                                                                                                                                                                               
sage: comp()                                                                                                                                                                        
2D Grid Graph for [5, 5]
8.11 µs ± 115 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
2D Grid Graph for [10, 10]
26.9 µs ± 64.3 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
2D Grid Graph for [50, 50]
827 µs ± 1.4 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
2D Grid Graph for [100, 100]
3.35 ms ± 5.38 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
2D Grid Graph for [500, 500]
115 ms ± 218 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
2D Grid Graph for [1000, 1000]
470 ms ± 882 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

After:

sage: def comp(): 
....:     for n in [5, 10, 50, 100, 500, 1000]: 
....:         G = graphs.Grid2dGraph(n, n)  
....:         print(G)  
....:         %timeit _ = list(G.depth_first_search(start=(0, 0))) 
....:                                                                                                                                                                               
sage: comp()                                                                                                                                                                        
2D Grid Graph for [5, 5]
4.41 µs ± 12.3 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
2D Grid Graph for [10, 10]
13.2 µs ± 27.5 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
2D Grid Graph for [50, 50]
409 µs ± 225 ns per loop (mean ± std. dev. of 7 runs, 1000 loops each)
2D Grid Graph for [100, 100]
1.67 ms ± 7.46 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
2D Grid Graph for [500, 500]
65.8 ms ± 54.2 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
2D Grid Graph for [1000, 1000]
264 ms ± 331 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

Depends on #31117

CC: @dcoudert

Component: graph theory

Author: Jonathan Kliem

Branch/Commit: 78d5536

Reviewer: David Coudert

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

@kliem kliem added this to the sage-9.3 milestone Dec 29, 2020
@kliem

This comment has been minimized.

@kliem
Copy link
Contributor Author

kliem commented Dec 29, 2020

Last 10 new commits:

156d222further improvement
e56743brevert last commit because some methods are not in all backends
e4415f9trac #31117: merged with 9.3.beta5
c342fe6trac #31117: better bfs
65e677eimplement next_out_neighbor for static sparse
b7e8a20Merge branch 'u/gh-kliem/next_out_neighbor_for_static_sparse' of git://trac.sagemath.org/sage into public/graphs/31117_BFS
eaf982buse next_out_neighbor_unsafe
4fd001ereuse queue
cc0cfd1account for that the loop is an if-clause now
606c8cfimprovements for depth_first_search

@kliem
Copy link
Contributor Author

kliem commented Dec 29, 2020

Commit: 606c8cf

@kliem
Copy link
Contributor Author

kliem commented Dec 29, 2020

Branch: public/31129

@dcoudert
Copy link
Contributor

comment:2

The errors reported by the patchbot are fixed in #31117. This ticket must be rebased to get last commit.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2020

Changed commit from 606c8cf to 78d5536

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 29, 2020

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

07eaaa5inlining makes a difference
c005179improvements for depth_first_search
78d5536trac #31117: fix next_in_neighbor_unsafe in static_sparse_backend

@dcoudert
Copy link
Contributor

comment:4

We have a green bot and it's much faster this way. Perfect !

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@kliem
Copy link
Contributor Author

kliem commented Dec 31, 2020

comment:5

Thank you.

@kliem
Copy link
Contributor Author

kliem commented Jan 2, 2021

comment:6

Btw, yield from also gives a significant improvement, I will do this in a follow up.

@vbraun
Copy link
Member

vbraun commented Jan 4, 2021

Changed branch from public/31129 to 78d5536

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

3 participants