Skip to content

Commit

Permalink
Trac #25872: Modular decomposition bug
Browse files Browse the repository at this point in the history
{{{
sage: G1 = Graph('FwA]w')
sage: G2 = Graph('F@Nfg')
sage: G1.modular_decomposition(algorithm='tedder')
(PRIME, [6, 2, 1, 5, 0, (PARALLEL, [3, 4])])
sage: G2.modular_decomposition(algorithm='tedder')
(PRIME, [6, 5, 0, 2, 3, 1, 4])
sage: G1.is_isomorphic(G2)
True
}}}

#24898 was not enough.

This ticket removes `'tedder'` implementation, as buggy unfixable
abandonware (see #33902 for another example of a bug);
the `algorithm=` option is removed, as we only have one implementation
now.

URL: https://trac.sagemath.org/25872
Reported by: jmantysalo
Ticket author(s): Dima Pasechnik
Reviewer(s): David Coudert, Matthias Koeppe
  • Loading branch information
Release Manager committed May 27, 2022
2 parents 5fb2a6e + f610e5a commit 70ec4e6
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 2,133 deletions.
95 changes: 35 additions & 60 deletions src/sage/graphs/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -7588,7 +7588,7 @@ def cores(self, k=None, with_labels=False):
return list(core.values())

@doc_index("Leftovers")
def modular_decomposition(self, algorithm='habib', style='tuple'):
def modular_decomposition(self, algorithm=None, style='tuple'):
r"""
Return the modular decomposition of the current graph.
Expand All @@ -7597,18 +7597,10 @@ def modular_decomposition(self, algorithm='habib', style='tuple'):
module or to none of them. Every graph that has a nontrivial module can
be partitioned into modules, and the increasingly fine partitions into
modules form a tree. The ``modular_decomposition`` function returns
that tree.
that tree, using an `O(n^3)` algorithm of [HM1979]_.
INPUT:
- ``algorithm`` -- string (default: ``'habib'``); specifies the
algorithm to use among:
- ``'tedder'`` -- linear time algorithm of [TCHP2008]_
- ``'habib'`` -- `O(n^3)` algorithm of [HM1979]_. This algorithm is
much simpler and so possibly less prone to errors.
- ``style`` -- string (default: ``'tuple'``); specifies the output
format:
Expand Down Expand Up @@ -7688,14 +7680,9 @@ def modular_decomposition(self, algorithm='habib', style='tuple'):
sage: g = graphs.CompleteGraph(5)
sage: g.add_edge(0,5)
sage: g.add_edge(0,6)
sage: g.modular_decomposition(algorithm='habib')
sage: g.modular_decomposition()
(SERIES, [(PARALLEL, [(SERIES, [1, 2, 3, 4]), 5, 6]), 0])
We get an equivalent tree when we use the algorithm of [TCHP2008]_::
sage: g.modular_decomposition(algorithm='tedder')
(SERIES, [(PARALLEL, [(SERIES, [4, 3, 2, 1]), 5, 6]), 0])
We can choose output to be a
:class:`~sage.combinat.rooted_tree.LabelledRootedTree`::
Expand All @@ -7712,39 +7699,33 @@ def modular_decomposition(self, algorithm='habib', style='tuple'):
ALGORITHM:
When ``algorithm='tedder'`` this function uses python implementation of
algorithm published by Marc Tedder, Derek Corneil, Michel Habib and
Christophe Paul [TCHP2008]_. When ``algorithm='habib'`` this function
uses the algorithm of M. Habib and M. Maurer [HM1979]_.
This function uses the algorithm of M. Habib and M. Maurer [HM1979]_.
.. SEEALSO::
- :meth:`is_prime` -- Tests whether a graph is prime.
- :class:`~sage.combinat.rooted_tree.LabelledRootedTree`.
.. NOTE::
A buggy implementation of linear time algorithm from [TCHP2008]_ was
removed in Sage 9.7, see :trac:`25872`.
TESTS:
Empty graph::
sage: graphs.EmptyGraph().modular_decomposition(algorithm='habib')
()
sage: graphs.EmptyGraph().modular_decomposition(algorithm='tedder')
sage: graphs.EmptyGraph().modular_decomposition()
()
sage: graphs.EmptyGraph().modular_decomposition(algorithm='habib', style='tree')
None[]
sage: graphs.EmptyGraph().modular_decomposition(algorithm='tedder', style='tree')
sage: graphs.EmptyGraph().modular_decomposition(style='tree')
None[]
Singleton Vertex::
sage: Graph(1).modular_decomposition(algorithm='habib')
sage: Graph(1).modular_decomposition()
(PRIME, [0])
sage: Graph(1).modular_decomposition(algorithm='tedder')
(PRIME, [0])
sage: Graph(1).modular_decomposition(algorithm='habib', style='tree')
PRIME[0[]]
sage: Graph(1).modular_decomposition(algorithm='tedder', style='tree')
sage: Graph(1).modular_decomposition(style='tree')
PRIME[0[]]
Vertices may be arbitrary --- check that :trac:`24898` is fixed::
Expand All @@ -7755,26 +7736,32 @@ def modular_decomposition(self, algorithm='habib', style='tuple'):
sage: sorted(md[1])
[(1, 2), (2, 3)]
Unknown algorithm::
sage: graphs.PathGraph(2).modular_decomposition(algorithm='abc')
Traceback (most recent call last):
...
ValueError: algorithm must be 'habib' or 'tedder'
Unknown style::
sage: graphs.PathGraph(2).modular_decomposition(style='xyz')
Traceback (most recent call last):
...
ValueError: style must be 'tuple' or 'tree'
Check that :trac:`25872` is fixed::
sage: G1 = Graph('FwA]w')
sage: G2 = Graph('F@Nfg')
sage: G1.is_isomorphic(G2)
True
sage: G1.modular_decomposition()
(PRIME, [1, 2, 5, 6, 0, (PARALLEL, [3, 4])])
sage: G2.modular_decomposition()
(PRIME, [5, 6, 3, 4, 2, (PARALLEL, [0, 1])])
"""
from sage.graphs.graph_decompositions.modular_decomposition import (modular_decomposition,
NodeType,
from sage.graphs.graph_decompositions.modular_decomposition import (NodeType,
habib_maurer_algorithm,
create_prime_node,
create_normal_node)

if algorithm is not None:
from sage.misc.superseded import deprecation
deprecation(25872, "algorithm=... parameter is obsolete and has no effect.")
self._scream_if_not_simple()

if not self.order():
Expand All @@ -7783,12 +7770,7 @@ def modular_decomposition(self, algorithm='habib', style='tuple'):
D = create_prime_node()
D.children.append(create_normal_node(self.vertices()[0]))
else:
if algorithm == 'habib':
D = habib_maurer_algorithm(self)
elif algorithm == 'tedder':
D = modular_decomposition(self)
else:
raise ValueError("algorithm must be 'habib' or 'tedder'")
D = habib_maurer_algorithm(self)

if style == 'tuple':
if D is None:
Expand Down Expand Up @@ -8049,23 +8031,13 @@ def is_inscribable(self, solver="ppl", verbose=0):
return self.planar_dual().is_circumscribable(solver=solver, verbose=verbose)

@doc_index("Graph properties")
def is_prime(self, algorithm='habib'):
def is_prime(self, algorithm=None):
r"""
Test whether the current graph is prime.
INPUT:
- ``algorithm`` -- (default: ``'tedder'``) specifies the algorithm to
use among:
- ``'tedder'`` -- Use the linear algorithm of [TCHP2008]_.
- ``'habib'`` -- Use the $O(n^3)$ algorithm of [HM1979]_. This is
probably slower, but is much simpler and so possibly less error
prone.
A graph is prime if all its modules are trivial (i.e. empty, all of the
graph or singletons) -- see :meth:`modular_decomposition`.
Use the `O(n^3)` algorithm of [HM1979]_.
EXAMPLES:
Expand All @@ -8086,12 +8058,15 @@ def is_prime(self, algorithm='habib'):
sage: graphs.EmptyGraph().is_prime()
True
"""
if algorithm is not None:
from sage.misc.superseded import deprecation
deprecation(25872, "algorithm=... parameter is obsolete and has no effect.")
from sage.graphs.graph_decompositions.modular_decomposition import NodeType

if self.order() <= 1:
return True

D = self.modular_decomposition(algorithm=algorithm)
D = self.modular_decomposition()

return D[0] == NodeType.PRIME and len(D[1]) == self.order()

Expand Down
Loading

0 comments on commit 70ec4e6

Please sign in to comment.