Opened 10 years ago
Last modified 7 years ago
#8433 reopened Feature Requests
Algorithm for finding all the elementary circuits in a directed (multi)graph
Reported by: | Louis Dionne | Owned by: | Jeremiah Willcock |
---|---|---|---|
Milestone: | To Be Determined | Component: | graph |
Version: | Boost 1.53.0 | Severity: | Not Applicable |
Keywords: | graph multigraph circuit cycle hawick | Cc: |
Description
I have implemented the algorithm described in (1) for finding all the elementary circuits of a directed (multi)graph. I implemented it in a generic fashion, planning to propose it for inclusion in Boost.
Boost.Graph currently contains the tiernan_all_cycles
algorithm for finding
all the cycles in a graph. However, it is undocumented and it has a time bound
(for which no precise analysis is provided) that makes it unsuitable even for
moderately sized graphs. In fact, I implemented hawick_circuits
because
tiernan_all_cycles
did not cut it for my application.
My implementation is available at (2). It is licensed under the Boost license and was tested using the test suite at (3). More information on the test suite can be found at (4).
Comments on improvements would be appreciated.
(1) http://www.massey.ac.nz/~kahawick/cstn/013/cstn-013.pdf
(2) https://github.com/ldionne/hawick_circuits
(3) https://github.com/josch/cycle_test
(4) http://blog.mister-muffin.de/2012/07/04/enumerating-elementary-circuits-of-a-directed_graph/
Change History (10)
follow-up: 2 comment:1 by , 10 years ago
comment:2 by , 9 years ago
Replying to jewillco:
Sorry, I did not notice your reply and now quite a bit of time has passed. Anyway, the algorithm is still a very relevant addition to Boost.Graph.
Here are a few comments on the code:
- It looks good and is almost ready to put in.
- There is no documentation file that I can see.
The documentation that I should write is almost the same as that of tiernan_all_cycles
. Is there a way to say "the behavior for hawick_circuits
is the same as tiernan_all_cycles
, except it detects self-loops and loops involving parallel edges in multigraphs"?
Also, what would be the preferred way of providing documentation? I am not familiar with QuickBook; if it's possible to stick with something else then I'd be happy.
- Please use
boost::graph_detail::find
from<boost/pending/container_traits.hpp>
instead ofstd::find
; that code will automatically use a member find to get better performance on types such asset
andunordered_set
. If you know that you are searching a non-associative container, you can also useboost::container_contains
from<boost/detail/algorithm.hpp>
.
I'm now using boost::container_contains
. I do find it awful that <boost/detail/algorithm.hpp>
includes so many unneeded headers, though.
- What concept is
ClosedMatrix
expected to model?
IIRC, ClosedMatrix
is just a two dimensional array of vertices. It is an implementation detail and it is not expected to model any specific concept.
- There is no need to use perfect forwarding on the graph type; passing
const
references is fine. You can also assume vertex descriptors and property maps are inexpensive to copy, but forwarding those is acceptable if you want to do it.
I like to use perfect forwarding whenever it makes sense to do it, i.e. whenever I don't use an argument but I only forward it to another function. While it may be useless in some cases, it is never harmful and I feel it captures my intent better.
- You refer to citation
[1]
in the code, but do not provide the full information about the source there.
Thanks, fixed.
- Boost.Graph has a Dot parser (and output routines) if you want to use them in your test harness.
Thanks, but the test harness is made to work with the test suite at https://github.com/josch/cycle_test. The input format is not exactly Dot.
I updated the repository at https://github.com/ldionne/hawick_circuits. I guess we have to settle on the documentation and then we'll be good to go.
follow-up: 4 comment:3 by , 9 years ago
Here are responses to your questions:
- Yes, you can have the documentation refer to another algorithm's documentation, but your page should at least have the function signatures and any differences. Most BGL documentation is in fairly straightforward, hand-written HTML; just copy one of the existing pages and modify it for your algorithm.
- It looks like
container_contains
does not do any optimizations, so you should probably useboost::graph_detail::find
anyway.
- That's fine; I see that
ClosedMatrix
isn't part of a public interface so it can be whatever you want.
- OK, perfect forwarding is fine as long as your code works in C++03.
- I'm not seeing exactly where the test graphs (or generators for them) are in the repository you give. How different are they from Graphviz format?
comment:4 by , 9 years ago
Replying to jewillco:
Here are responses to your questions:
- Yes, you can have the documentation refer to another algorithm's documentation, but your page should at least have the function signatures and any differences. Most BGL documentation is in fairly straightforward, hand-written HTML; just copy one of the existing pages and modify it for your algorithm.
I wrote documentation in Markdown and generated HTML from it. It looks exactly as the rest of the documentation.
- It looks like
container_contains
does not do any optimizations, so you should probably useboost::graph_detail::find
anyway.
Since I'm only searching in a vector, there won't be any optimization. I'll stick with container_contains
, since that expresses my intent better.
- OK, perfect forwarding is fine as long as your code works in C++03.
The code works in C++03 and C++11. It was compiled with Clang 3.3 and G++ 4.9.
- I'm not seeing exactly where the test graphs (or generators for them) are in the repository you give. How different are they from Graphviz format?
I'm not providing any tests with the algorithm, that's why. The algorithm is tested using an external test suite, and it would be fairly complicated to migrate those tests to Boost. Basically, you should ignore the hawick_circuits.cpp
file at the root of the repository, which is only useful for the external test suite.
follow-up: 6 comment:5 by , 9 years ago
Instead of container_contains
, it would probably be better to use std::find
directly (which I believe you did before). Are you going to have any kind of testing in the Boost version? It would be nice to at least have testing with a small graph or two that makes sure that the code compiles and can run simple problems. Can you use random graphs like the tiernan_all_cycles.cpp
test does?
comment:6 by , 9 years ago
Replying to jewillco:
Instead of
container_contains
, it would probably be better to usestd::find
directly (which I believe you did before).
Fixed.
Are you going to have any kind of testing in the Boost version? It would be nice to at least have testing with a small graph or two that makes sure that the code compiles and can run simple problems. Can you use random graphs like the
tiernan_all_cycles.cpp
test does?
I adapted tiernan_all_cycles.cpp
into a reusable header for cycle algorithms, and used it to generate a small unit test for hawick_circuits
. It might also be interesting to use that header from tiernan_all_cycles.cpp
to reduce code duplication.
comment:7 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:8 by , 9 years ago
I've added your code to the Boost trunk, but I still need a couple of changes (please attach patches to the bug report for those):
- The documentation files do not include copyright or license information; could you please send a patch with that added?
- Where should your algorithm be in the table of contents of the documentation? I put it in the "Miscellaneous Algorithms" part for now.
Also, here are some changes I made; please tell me if you disagree:
- I put the
hawick_circuits.cpp
file from the root directory of your Github repository intolibs/graph/example
and added it as a file to build there. - I removed two lines (
typedef
s) fromcycle_test.hpp
to fix GCC warnings about unusedtypedef
s.
comment:9 by , 8 years ago
Component: | graph → xpressive |
---|---|
Milestone: | To Be Determined → Website 1.X |
Resolution: | fixed |
Status: | closed → reopened |
Type: | Feature Requests → Library Submissions |
Version: | Boost 1.54.0 → Boost.Build-M3 |
I'm truly enjoying the design and layout of your blog. It's a very easy on the eyes which makes it much more pleasant for me to come here and visit more often. Did you hire out a developer to create your theme? Outstanding work! effckkaddggaecee
comment:10 by , 7 years ago
Component: | xpressive → graph |
---|---|
Milestone: | Website 1.X → To Be Determined |
Type: | Library Submissions → Feature Requests |
Version: | Boost.Build-M3 → Boost 1.53.0 |
Damn bot. We should be good to close this once https://github.com/boostorg/graph/pull/50 is merged.
Here are a few comments on the code:
boost::graph_detail::find
from<boost/pending/container_traits.hpp>
instead ofstd::find
; that code will automatically use a member find to get better performance on types such asset
andunordered_set
. If you know that you are searching a non-associative container, you can also useboost::container_contains
from<boost/detail/algorithm.hpp>
.ClosedMatrix
expected to model?const
references is fine. You can also assume vertex descriptors and property maps are inexpensive to copy, but forwarding those is acceptable if you want to do it.[1]
in the code, but do not provide the full information about the source there.