Module and Graph method for Projective planarity via forbidden minors #39802
Conversation
src/sage/graphs/graph.py
Outdated
| @@ -9235,6 +9235,60 @@ def bipartite_double(self, extended=False): | |||
| G.name("%sBipartite Double of %s" % (prefix, self.name())) | |||
| return G | |||
|
|
|||
| @doc_index("Graph properties") | |||
| def is_projective_planar(self, minor_map=False, **minor_kwargs): | |||
There was a problem hiding this comment.
this method could be defined in file projective_planarity.py and then imported in Graph.
There was a problem hiding this comment.
@dcoudert Do you mean that we should define a wrapper of some sort in the Graphs class that uses the method is_projective_planar?
There was a problem hiding this comment.
To see how methods are imported in Graph and declared in the documentation, check file graph.py from line 9240 to 9354
src/sage/graphs/graph.py
Outdated
| p2_forbidden_minor = get_p2_forbidden_minor(self, **minor_kwargs) | ||
| if minor_map: | ||
| return p2_forbidden_minor | ||
| else: |
There was a problem hiding this comment.
the else is not needed here.
| @@ -0,0 +1,416 @@ | |||
| r""" | |||
| Minimal forbidden minors for projective plane and function for finding them in G | |||
There was a problem hiding this comment.
This module should be names Projective planarity. Then you have text to explain what this is about.
There was a problem hiding this comment.
@dcoudert Can you elaborate on this comment? We don't understand what you're trying to say.
There was a problem hiding this comment.
The first line will be displayed in the documentation as the title of the module. Mu suggèstion is to change to
r"""
Projective planarity
This module contains the 35 minimal forbidden minors for projective plane
and a function for checking if a graph `G` has one of them as a minor.| # https://www.gnu.org/licenses/ | ||
| # **************************************************************************** | ||
|
|
||
| from copy import deepcopy |
There was a problem hiding this comment.
I don"t think you need it.
| def get_p2_forbidden_minor(G, **minor_kwargs): | ||
| """ | ||
| Check if one of the minimal forbidden minors of the projective plane is an | ||
| induced minor of G. |
|
|
||
| OUTPUT: | ||
|
|
||
| Return :meth:`~Graph.minor` output if an element of P2_FORBIDDEN_MINORS is |
There was a problem hiding this comment.
P2_FORBIDDEN_MINORS -> P2_FORBIDDEN_MINORS
|
|
||
| EXAMPLES: | ||
|
|
||
| #. The Peterson graph is a known projective planar graph so it doesn't have |
|
|
||
| for forbidden_minor in P2_FORBIDDEN_MINORS: | ||
| # Can't be a minor if it has more vertices or edges than G | ||
| if ( |
There was a problem hiding this comment.
if (forbidden_minor.num_verts() > num_verts_G
or forbidden_minor.num_edges() > num_edges_G):| ): | ||
| continue | ||
|
|
||
| try: |
There was a problem hiding this comment.
why using a try...except statement ?
There was a problem hiding this comment.
G.minor throws a ValueError if a minor is not found
|
Documentation preview for this PR (built with commit 2849c51; changes) is ready! 🎉 |
|
@dcoudert can you comment on how we should declare this file in meson.build? It's just a python file with no cython or c to go with it.
|
open file |
|
Can you help us avoid the circular import problem? These lines appear in projective_planarity.py, and we're trying to keep the bulk of the code for the projective planarity check in a separate file. from sage.graphs.graph import Graph We could also presumably put the code in the graph.py file, but that's presumably not the way you want it done. |
src/sage/graphs/graph.py
Outdated
| @@ -9236,13 +9238,10 @@ def bipartite_double(self, extended=False): | |||
| return G | |||
|
|
|||
| @doc_index("Graph properties") | |||
| def is_projective_planar(self, minor_map=False, **minor_kwargs): | |||
| def is_projective_planar(self, minor_map=False): | |||
There was a problem hiding this comment.
parameter minor_map should be renamed return_map. Currently, you mix the input parameter with the internal variable minor_map storing the result of the call to self.minor(..).
src/sage/graphs/graph.py
Outdated
| r""" | ||
| Check whether this graph is projective planar. | ||
|
|
||
| Wraps :func:`get_p2_forbidden_minor |
There was a problem hiding this comment.
add a few lines to explain what it means to be projective planar.
src/sage/graphs/graph.py
Outdated
| num_edges_G = self.num_edges() | ||
| minor_map = None | ||
|
|
||
| for forbidden_minor_string in P2_FORBIDDEN_MINORS: |
There was a problem hiding this comment.
a better form:
for forbidden_minor_string in P2_FORBIDDEN_MINORS:
# Can't be a minor if it has more vertices or edges than G
forbidden_minor = Graph(forbidden_minor_string)
if (forbidden_minor.num_verts() > num_verts_G
or forbidden_minor.num_edges() > num_edges_G):
continue
try:
minor_map = self.minor(forbidden_minor)
if minor_map is not None:
break
# If self has no H minor, then self.minor(H) throws a ValueError
except ValueError:
continue
if return_map:
return minor_map
return minor_map is None| @@ -0,0 +1,62 @@ | |||
| r""" | |||
| p2_forbidden_minors | |||
There was a problem hiding this comment.
What's the need of this file ? why not simply adding the list to method is_projective_planar ?
Do you have any reference with the proof that this list is the correct list of minors ?
There was a problem hiding this comment.
Thank you for your helpful comments!
The short answer is yes, we do have a reference.
The book we used is this one.
B. Mohar and C. Thomassen, Graphs on Surfaces, The Johns Hopkins University Press, 2001.
Theorem 6.5.1, pages 197-198.
The longer answer is that we used the existing sage codebase to hard coded the graphs in that theorem using and then put them in graph6 format.
How would you like us to say this in our code?
We thought it might be helpful to leave the list of excluded minors in a separate file in import them. In the original hard coded form (not using graph6), it's a lot more code. Please tell us what you'd like us to do: shall we leave them in graph6 format, or leave them in a longer hardcoded format that uses the existing sage codebase.
There was a problem hiding this comment.
We have for instance method graphs.petersen_family that returns a collection of 7 graphs which are the forbidden minors of the linklessly embeddable graphs.
See: https://doc.sagemath.org/html/en/reference/graphs/sage/graphs/generators/families.html#sage.graphs.generators.families.petersen_family
and file src/sage/graphs/generators/families.py
A similar method could be added for the p2 forbidden minors. This would allow the graphs to be accessed for different purposes. The method could simply contain the list of graph6 strings and produce them as graphs. If the graphs have names, these can be specified. There is no need to specify the embedding of each graph.
There was a problem hiding this comment.
The reference can be added to file src/doc/en/reference/references/index.rst. Then you can refer to it using [MT2001]_ (assuming that the key if MT2001.
src/sage/graphs/graph.py
Outdated
| True | ||
|
|
||
| `K_{4,4}` has a projective plane crossing number of 2. One of the | ||
| minimal forbidden minors is `K_{4,4} - e`, so we get a one-to-one |
There was a problem hiding this comment.
the alignment is incorrect.
- `K_{4,4}` has a projective plane crossing number of 2. One of the
- minimal forbidden minors is `K_{4,4} - e`, so we get a one-to-one
- dictionary from :meth:`~Graph.minor`::
+ `K_{4,4}` has a projective plane crossing number of 2. One of the
+ minimal forbidden minors is `K_{4,4} - e`, so we get a one-to-one
+ dictionary from :meth:`~Graph.minor`::
src/sage/graphs/graph.py
Outdated
|
|
||
| TESTS:: | ||
|
|
||
| sage: len(graphs. p2_forbidden_minors()) |
There was a problem hiding this comment.
no space between . and p2_forbidden_minors
|
@dimpase, can you try again to trigger CI runs ? |
There was a problem hiding this comment.
Please be more careful. Currently doctests are failing, we have lint errors, etc.
Here are all the issues that remains to be fixed in the code:
diff --git a/src/sage/graphs/generators/families.py b/src/sage/graphs/generators/families.py
index 69a17f182dc..2cd519d6762 100644
--- a/src/sage/graphs/generators/families.py
+++ b/src/sage/graphs/generators/families.py
@@ -3132,6 +3132,10 @@ def p2_forbidden_minors():
We return the graphs using the Sage Graph constructor.
+ TESTS::
+
+ sage: len(graphs. p2_forbidden_minors())
+ 35
"""
p2_forbidden_minors_graph6 = [
diff --git a/src/sage/graphs/graph.py b/src/sage/graphs/graph.py
index 14b253e1e31..602d5a1fe91 100644
--- a/src/sage/graphs/graph.py
+++ b/src/sage/graphs/graph.py
@@ -9522,7 +9522,7 @@ class Graph(GenericGraph):
plane. The approach is to check that the graph does not contain any
of the known forbidden minors.
- INPUT
+ INPUT:
- ``return_map`` -- boolean (default: ``False``); whether to return
a map indicating one of the forbidden graph minors if in fact the
@@ -9540,32 +9540,26 @@ class Graph(GenericGraph):
The Peterson graph is a known projective planar graph::
sage: P = graphs.PetersenGraph()
- sage: P.is_projective_planar()
+ sage: P.is_projective_planar() # long time
True
`K_{4,4}` has a projective plane crossing number of 2. One of the
- minimal forbidden minors is `K_{4,4} - e`, so we get a one-to-one
- dictionary from :meth:`~Graph.minor`::
+ minimal forbidden minors is `K_{4,4} - e`, so we get a one-to-one
+ mapping from :meth:`~Graph.minor`::
sage: K44 = graphs.CompleteBipartiteGraph(4, 4)
- sage: minor_map = K44.is_projective_planar()
- sage: minor_map
- {0: [0], 1: [1], 2: [2], 3: [3], 4: [4], 5: [5], 6: [6], 7: [7]}
+ sage: K44.is_projective_planar(return_map=True)
+ (False,
+ {0: [0], 1: [1], 2: [2], 3: [3], 4: [4], 5: [5], 6: [6], 7: [7]})
.. SEEALSO::
- :meth:`~Graph.minor`
- TESTS::
-
- sage: len(graphs.p2_forbidden_minors())
- 35
"""
-
from sage.graphs.generators.families import p2_forbidden_minors
num_verts_G = self.num_verts()
num_edges_G = self.num_edges()
- minor_map = None
for forbidden_minor in p2_forbidden_minors():
# Can't be a minor if it has more vertices or edges than G
@@ -9579,8 +9573,7 @@ class Graph(GenericGraph):
if minor_map is not None:
if return_map:
return False, minor_map
- else:
- return False
+ return False
# If G has no H minor, then G.minor(H) throws a ValueError
src/sage/graphs/graph.py
Outdated
| plane. The approach is to check that the graph does not contain any | ||
| of the known forbidden minors. | ||
|
|
||
| INPUT |
There was a problem hiding this comment.
you are missing : after INPUT. It should be INPUT:
|
My main concern is that method sage: P = graphs.PetersenGraph()
sage: %time P.is_projective_planar()
CPU times: user 2min 51s, sys: 642 ms, total: 2min 52s
Wall time: 2min 52s
True
sage: K44 = graphs.CompleteBipartiteGraph(4, 4)
sage: %time K44.is_projective_planar(return_map=True)
CPU times: user 2.12 s, sys: 11.5 ms, total: 2.13 s
Wall time: 2.13 s
(False, {0: [0], 1: [1], 2: [2], 3: [3], 4: [4], 5: [5], 6: [6], 7: [7]}) |
|
can this be computed via combinatorial embeddings? |
|
I don't know of any other implemented projective planarity algorithm. This work is related to a research project that @juan-mlr and I are doing, which is a continuation of https://combinatorialpress.com/article/jcmcc/Some-Excluded-Minors-for-the-Spindle-Surface.pdf for the projective plane. Thank you very much for your thoughtfulness. |
|
https://doc.sagemath.org/html/en/reference/graphs/sage/graphs/genus.html lets you compute the (oriented) genus of a graph, and it's reasonably fast on small graphs. |
|
There is also this paper by Mohar https://doi.org/10.1006/jagm.1993.1050 https://www.sfu.ca/~mohar/Reprints/1993/BM93_JA15_Mohar_ProjectivePlanarity.pdf Let us finish this PR. We can let the implementation of a faster method in the TODO list. |
|
Is there anything else we need to do? @dcoudert |
|
|
||
| - :meth:`~Graph.minor` | ||
|
|
||
| TESTS:: |
There was a problem hiding this comment.
Add a similar test block in method graphs.p2_forbidden_minors
src/sage/graphs/graph.py
Outdated
|
|
||
| OUTPUT: | ||
|
|
||
| Return True if the graph is projective planar and False if not. If the |
There was a problem hiding this comment.
correct form so that the documentation can be build properly and display well
Return ``True`` if the graph is projective planar and ``False`` if not. If the
parameter ``map_flag`` is ``True`` and the graph is not projective planar, then
the method returns ``False`` and a map from :meth:`~Graph.minor`
indicating one of the forbidden graph minors.
There was a problem hiding this comment.
Thank you @dcoudert. We'll get it done.
There was a problem hiding this comment.
Thank you again @dcoudert . We have acted on your comments.
|
@dcoudert thank you for staying with me on this PR through each commit. I'd like to see the Mojar paper get implemented. Perhaps I'll have time in the future. That sounds like a nice challenge. |
|
|
For some reason, the code of method Sorry Volker for not seing that before. Not easy to work without CIs. |
|
I'm sorry about the mistake. I should have caught that one. I've made another push. Thank you for your comments and support. |
Addresses #37937 (to enable checking if a graph in projective planar) by
src/sage/graphs/projective_planarity.py, that holds the relevant forbidden minors and a function to search for them in another graph.is_projective_planarmethod toGraph.📝 Checklist
⌛ Dependencies