compute fundamental groups of simplicial complexes

jhpalmieri commented 12 years ago

The attached patch provides a fundamental_group method for simplicial complexes.

Depends on #12339

Component: algebraic topology

Author: John Palmieri

Reviewer: Miguel Marco

Merged: sage-5.8.beta1

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

jhpalmieri commented 12 years ago

Dependencies: #12339

fchapoton commented 11 years ago

comment:2

one comment : the last part of fundamental_group, where you use many nested tests to distinguish cases, does not look good and is hard to read. Maybe rather do something like

z = dict()
for i in range(3):
    if bdry[i] in spanning_tree:
        z[i] = FG.one()
    else:
        z[i] = FG.gen(gens_dict[bdry[i]])

and then

rels.append(z[0]*z[1].inverse()*z[2])

?

miguelmarco commented 11 years ago

comment:3

I think the suggestion of chapoton makes a lot of sense. Besides that, it looks good. I would be happy to give a positive review once the minor change is made.

miguelmarco commented 11 years ago

Reviewer: mmarco

jhpalmieri commented 11 years ago

comment:5

Thank you for the suggestions. It may be a few weeks before I have time to implement anything.

jhpalmieri commented 11 years ago

Attachment: trac_13254-pi_1.patch.gz

jhpalmieri commented 11 years ago

comment:6

Okay, here's a new version of the patch, with chapoton's suggested simpler code for computing the relations.

miguelmarco commented 11 years ago

comment:7

Looks good to me. By checking this i discovered something the shouldn't happen, but it has nothing to do with this ticket:

sage: F=simplicial_complexes.Sphere(0)
sage: F.connected_component()
---------------------------------------------------------------------------
LookupError                               Traceback (most recent call last)
<ipython-input-48-5cd541b6e495> in <module>()
----> 1 F.connected_component()

/home/mmarco/sage-5.7.beta3/local/lib/python2.7/site-packages/sage/homology/simplicial_complex.pyc in connected_component(self, simplex)
   2911         else:
   2912             v = simplex[0]
-> 2913         vertices = self.graph().connected_component_containing_vertex(v)
   2914         facets = [f for f in self.facets() if f.is_face(Simplex(vertices))]
   2915         return SimplicialComplex(facets)

/home/mmarco/sage-5.7.beta3/local/lib/python2.7/site-packages/sage/graphs/generic_graph.pyc in connected_component_containing_vertex(self, vertex)
   3894         """
   3895         try:
-> 3896             c = list(self._backend.depth_first_search(vertex, ignore_direction=True))
   3897         except AttributeError:
   3898             c = list(self.depth_first_search(vertex, ignore_direction=True))

/home/mmarco/sage-5.7.beta3/local/lib/python2.7/site-packages/sage/graphs/base/c_graph.so in sage.graphs.base.c_graph.CGraphBackend.depth_first_search (sage/graphs/base/c_graph.c:17312)()

/home/mmarco/sage-5.7.beta3/local/lib/python2.7/site-packages/sage/graphs/base/c_graph.so in sage.graphs.base.c_graph.Search_iterator.__init__ (sage/graphs/base/c_graph.c:19227)()

LookupError: Vertex (0) is not a vertex of the graph.

jdemeyer commented 11 years ago

Changed reviewer from mmarco to Miguel Marco

jhpalmieri commented 11 years ago

Attachment: trac_13254-vertex-fix.patch.gz

jhpalmieri commented 11 years ago

comment:11

Since the milestone for this ticket has been pushed back to 5.8, we can fix the problem in comment 7 here, too. It's a one line fix: line 2543 in the new version. The rest of the patch is just modifications to doctests. It is possible that the various warnings like

This may give the wrong answer if the simplicial complex 
was constructed with ``maximality_check`` set to ``False``.

may not be true any more, but I'm not sure. Since they say "This may give the wrong answer ...", I think we can leave them intact. People should know to be careful when constructing simplicial complexes with maximality_check=False.

tscrim commented 11 years ago

comment:12

Looks good to me.

jdemeyer commented 11 years ago

Merged: sage-5.8.beta1

sagemath / sage

compute fundamental groups of simplicial complexes #13254