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

acyclic_edge_coloring(Graph(3), k=None, value_only=True) should not be 2, should it? #27079

Closed
mantepse opened this issue Jan 18, 2019 · 17 comments

Comments

@mantepse
Copy link
Collaborator

I think that the correct value for

acyclic_edge_coloring(Graph(3), k=None, value_only=True)

should be 0, since there is nothing to color.

Component: graph theory

Author: David Coudert

Branch/Commit: 2996f7a

Reviewer: Martin Rubey

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

@mantepse mantepse added this to the sage-8.7 milestone Jan 18, 2019
@dcoudert
Copy link
Contributor

comment:1

As documented, this method computes by default a coloring of G into \Delta(G) + 2 colors. So the result you get is at least consistent with the documentation.

For the same reason, we get the following:

sage: acyclic_edge_coloring(graphs.PathGraph(2), value_only=True)
3

But we can check that a coloring with 1 color exists

sage: acyclic_edge_coloring(graphs.PathGraph(2), k=1, value_only=True)
1

The current implementation makes it impossible to return 0. So tests for small cases must be added.

@mantepse

This comment has been minimized.

@mantepse
Copy link
Collaborator Author

comment:3

Indeed, I made a mistake in the description of the ticket. I corrected it now, thank you!

@mantepse mantepse changed the title acyclic_edge_coloring(Graph(3), value_only=True) should not be 2, should it? acyclic_edge_coloring(Graph(3), k=None, value_only=True) should not be 2, should it? Jan 19, 2019
@dcoudert
Copy link
Contributor

comment:4

This should fix the reported issue.


New commits:

7fcd8aetrac #27079: edgeless graph

@dcoudert
Copy link
Contributor

Commit: 7fcd8ae

@dcoudert
Copy link
Contributor

Author: David Coudert

@dcoudert
Copy link
Contributor

Branch: public/27079_edgeless_graphs

@mantepse
Copy link
Collaborator Author

comment:5

Looks good, thank you! I think it would also be good to change the behaviour of the parameter k slightly, perhaps using k=-1 as default? But not in this ticket!

Finally, I am not sure: should the empty graph also be allowed as input?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2019

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

0a9b292trac #27079: empty graph

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2019

Changed commit from 7fcd8ae to 0a9b292

@dcoudert
Copy link
Contributor

comment:7

Right, the empty graph was leading to an error. This is now fixed.

I agree that change in default behavior, if done, should be in a specific ticket.

@mantepse
Copy link
Collaborator Author

comment:8

I am very sorry, but I have to ask:

The documentation currently says:

    - ``k`` -- integer; the number of colors to use.

      - If ``k > 0``, computes an acyclic edge coloring using `k` colors.

      - If ``k = 0`` (default), computes a coloring of `G` into `\Delta(G) + 2`
        colors, which is the conjectured general bound.

Apparently, when k is larger than the acyclic chromatic index of G, the function still returns a list of k graphs, some simply do not contain any edges.

Here is a proposal:

diff --git a/src/sage/graphs/graph_coloring.py b/src/sage/graphs/graph_coloring.py
index 9f0c6e47da..5b571a6eca 100644
--- a/src/sage/graphs/graph_coloring.py
+++ b/src/sage/graphs/graph_coloring.py
@@ -1546,17 +1546,21 @@ def acyclic_edge_coloring(g, hex_colors=False, value_only=False, k=0, solver=Non
 
     from sage.rings.integer import Integer
     from sage.combinat.subset import Subsets
+    from sage.plot.colors import rainbow
 
     if not g.order() or not g.size():
         if value_only:
             if k is None:
                 return 0
-            elif k == 0:
+            if k == 0:
                 return 2
-            else:
-                return k
+            return k
         else:
-            return {} if hex_colors else []
+            if k is None:
+                return {} if hex_colors else []
+            if k == 0:
+                k = 2
+            return {c: [] for c in rainbow(k)} if hex_colors else [g]*k
 
     if k is None:
         k = max(g.degree())
@@ -1577,8 +1581,6 @@ def acyclic_edge_coloring(g, hex_colors=False, value_only=False, k=0, solver=Non
     elif not k:
         k = max(g.degree()) + 2
 
-    from sage.plot.colors import rainbow
-
     p = MixedIntegerLinearProgram(solver=solver)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2019

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

2996f7atrac #27079: further improvements

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 19, 2019

Changed commit from 0a9b292 to 2996f7a

@dcoudert
Copy link
Contributor

comment:10

I have almost followed your proposal.

@mantepse
Copy link
Collaborator Author

Reviewer: Martin Rubey

@vbraun
Copy link
Member

vbraun commented Jan 26, 2019

Changed branch from public/27079_edgeless_graphs to 2996f7a

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