-
Notifications
You must be signed in to change notification settings - Fork 212
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
new planar_vertex_six_coloring() algorithm with examples, tests and doc #387
base: develop
Are you sure you want to change the base?
Conversation
I need help. Who will resolve these issues outside of my PR? Examples from:
The CI seems not to know that it should not test below C++14. https://www.boost.org/users/history/version_1_86_0.html
|
C++11 CI jobs are launched due to lines like this: https://github.com/boostorg/graph/blob/develop/.drone.star#L17 If you remove those lines, those jobs will disappear. |
Thanks, and sorry, I'm a bit behind with updating the CI configuration. |
I fixed |
Now, I forgot to say: thank you for a most interesting algorithm proposal. I'm not certain, but I imagine most algorithms and data structures in Boost.Graph are based on published literature, so although I am quite happy and even excited to accept novel algorithms and data structures, there is a higher burden of proof on the part of the contributor to demonstrate good theoretical design. It will take some time to evaluate, and if there are any issues found then it may require several rounds of review and evolution, but don't be despondent, that's how we (try to) maintain high quality. |
Sorry that I did not mention the literature, it is: Marek Chrobak, David Eppstein Without paywall from author: This code implements the determination of 5-bounded acyclic orientation of G, the order of vertices visited is in vector "visited": Before Finally Now that you opened the discussion, the first two parts just determine the order for coloring the vertices with at most 6 colors. After having submitted I realized that the 3rd part can be replaced with a single call to sequential_vertex_coloring() with passing the order determined as 3rd argument: And add a comment "determine 5-bounded acyclic orientation" with link to literature for 2nd part. |
I asked myself whether new class undirected_graph_constant_time_edge_add_and_remove is really needed, and it is. Reason is that from user code only iterator of a vertices out_edges() can be accessed, but not the out_edges list itself. That list would be needed for calling std::list erase() from user code. |
@jeremy-murphy |
I've noticed a couple of odd things about the CI lately, I might have to ask someone with more intimate knowledge of it to have a look. |
Btw I don't think I can re-run the CI on the same commit. I think it requires a new commit to trigger it, or possibly closing and reopening the PR might work. |
That's a transient error. |
@jeremy-murphy if you can't rerun Drone builds, ask @sdarwin to give you the necessary permissions. |
@jeremy-murphy @grisumbras |
I don't think there was a rerun. If you follow the Drone link, you can see that the build is from the 2nd of November. |
After re-running, the job succeeded. |
@grisumbras @jeremy-murphy Before I do the changes proposed, I want to ask you whether I should do the code improvement or not. These 3 files are not needed anymore:
The changes in these two deep graph files can be reverted as they are not needed anymore:
The pull request would then consist of these 9 files only:
So how does the simpification work? There is only one use of "g.impl()" in new visit_edge() that removes an edge in O(1) Use of "g.impl()" is essential, without no O(1) remove_edge is possible. https://en.cppreference.com/w/cpp/iterator/prev I tried to use "std::prev()" in line 49, but then the code just hangs indefinitely, so I cannot use it? Thanks in advance for your help. |
Thanks, Sam! |
@sdarwin, is that possible? It would sure come in handy some times. |
Most drone permissions are based on github.
The admin permissions will carry over. |
Why planar_vertex_six_coloring() is useful can be seen in the first of three tests for that algorithm.$k$ colors for any $k$ (k=15 in test).
The existing sequential_vertex_coloring() can be forced to use
Planar graphs can always be 4-colored, but algorithm for that has quadratic runtime.
planar_vertex_six_coloring() has linear runtime.
For that runtime a copy of the graph needs to be used, that allows for constant time edge removal.
For that purpose new class undirected_graph_constant_time_edge_add_and_remove derived from undirected_graph was implemented.
To do its job a new protected member function remove_edge_() was added to undirected graph that allows for O(1) edge removal if both out_edge_iterator of an edge are passed (which the new class does).
The example for undirected_graph_constant_time_edge_add_and_remove does demonstrate quadratic runtime of undirected graph for a task needed for 6-coloring algorithm, while new class shows only linear time for that:
The planar_vertex_six_coloring() example creates a random maximal planar graph with 1million edges, and 6-colors it in few seconds:
Finally the test of new algorithm does compare sequential_vertex_coloring() and planar_vertex_six_coloring() on
Without arguments passed it is just the needed test, with arguments "15 output" it does the same as test does and generates output:
This is my first pull request for boost.
So I tried to provide clean code, with examples and test.
And with new documentation for the derived class and the new algorithm.
I used existing doc for undirected_graph and sequential_vertex_coloring as basis for new doc.