Fix FinitelyPresentedGroupElement.__hash__

src/sage/groups/finitely_presented.py

@@ -351,7 +351,7 @@ def __call__(self, *values, **kwds):
             ...
             ValueError: the values do not satisfy all relations of the group
             sage: w(1, 2, check=False)    # result depends on presentation of the group element
-            2
+            8
         """
         values = list(values)
         if kwds.get('check', True):
@@ -361,6 +361,57 @@ def __call__(self, *values, **kwds):
                     raise ValueError('the values do not satisfy all relations of the group')
         return super().__call__(values)
 
+    def _normal_form_by_gap_nffunction(self) -> GapElement:
+        """
+        Internal method that calls ``FpElementNFFunction`` to compute some normal form
+        for this group element.
+        Note that this is not guaranteed to terminate.
+
+        .. SEEALSO::
+
+            Another way to compute a normal form is to use
+            :meth:`FinitelyPresentedGroup.confluent_rewriting_system`.
+
+        OUTPUT:
+
+        A :class:`~GapElement` such that if ``x == y``
+        then ``x._normal_form_by_gap_nffunction()`` is
+        determined only by the value of this group element,
+        and not the Tietze list.
+
+        TESTS::
+
+            sage: F.<a,b> = FreeGroup()
+            sage: G = F / [a*b/a/b]
+            sage: G(a*b) == G(b*a)
+            True
+            sage: G(a*b).Tietze() == G(b*a).Tietze()
+            False
+            sage: G(a*b)._normal_form_by_gap_nffunction() == G(b*a)._normal_form_by_gap_nffunction()
+            True
+        """
+        x = self.gap()
+        return x.FamilyObj().FpElementNFFunction()(x.UnderlyingElement())
+
+    def __hash__(self) -> int:
+        """
+        Return some hash value, such that if ``x == y``
+        then ``hash(x) == hash(y)``.
+        Note that this is not guaranteed to terminate.
+
+        TESTS::
+
+            sage: F.<a,b> = FreeGroup()
+            sage: G = F / [a*b/a/b]
+            sage: G(a*b) == G(b*a)
+            True
+            sage: G(a*b).Tietze() == G(b*a).Tietze()
+            False
+            sage: hash(G(a*b)) == hash(G(b*a))
+            True
+        """
+        return hash(self._normal_form_by_gap_nffunction())
+
 
 class RewritingSystem:
     """
@@ -536,12 +587,23 @@ def reduce(self, element):
     def gap(self):
         """
         The gap representation of the rewriting system.
+        Prefer to use ``libgap(k)`` for consistency.
+        The returned object should not be mutated, otherwise with the current implementation,
+        ``self`` will be mutated as well.
 
         EXAMPLES::
 
             sage: F.<a,b> = FreeGroup()
             sage: G = F/[a*a,b*b]
             sage: k = G.rewriting_system()
+            sage: libgap(k)
+            Knuth Bendix Rewriting System for Monoid( [ a, A, b, B ] ) with rules
+            [ [ a*A, <identity ...> ], [ A*a, <identity ...> ],
+              [ b*B, <identity ...> ], [ B*b, <identity ...> ],
+              [ a^2, <identity ...> ], [ b^2, <identity ...> ] ]
+
+        TESTS::
+
             sage: k.gap()
             Knuth Bendix Rewriting System for Monoid( [ a, A, b, B ] ) with rules
             [ [ a*A, <identity ...> ], [ A*a, <identity ...> ],
@@ -550,6 +612,8 @@ def gap(self):
         """
         return self._gap
 
+    _libgap_ = gap
+
     def rules(self):
         """
         Return the rules that form the rewriting system.
@@ -647,7 +711,7 @@ def make_confluent(self):
 
         .. WARNING::
 
-            This algorithm is not granted to finish. Although it may be useful
+            This algorithm is not guaranteed to finish. Although it may be useful
             in some occasions to run it, interrupt it manually after some time
             and use then the transformed rewriting system. Even if it is not
             confluent, it could be used to reduce some words.
@@ -680,10 +744,7 @@ def make_confluent(self):
                 b*a    --->    a*b^-1
                 b^2    --->    b^-1
         """
-        try:
-            self._gap.MakeConfluent()
-        except ValueError:
-            raise ValueError('could not make the system confluent')
+        self._gap.MakeConfluent()
 
 
 @richcmp_method
@@ -1873,7 +1934,7 @@ def characteristic_varieties(self, ring=QQ, matrix_ideal=None, groebner=False):
             j += 1
         return char_var
 
-    def rewriting_system(self):
+    def rewriting_system(self) -> RewritingSystem:
         """
         Return the rewriting system corresponding to the finitely presented
         group. This rewriting system can be used to reduce words with respect
@@ -1908,4 +1969,35 @@ def rewriting_system(self):
         """
         return RewritingSystem(self)
 
+    @cached_method
+    def confluent_rewriting_system(self) -> RewritingSystem:
+        """
+        Return some confluent rewriting system for this group.
+
+        .. NOTE::
+
+            - The returned rewriting system should not be mutated.
+
+            - The returned rewriting system may be different when Sage is restarted,
+              or for a different (but equal) group. For the same group object,
+              it is guaranteed to be the same.
+
+            - This function is not guaranteed to terminate.
+
+        EXAMPLES::
+
+            sage: F.<a,b> = FreeGroup()
+            sage: G = F/[a*a,b*b]
+            sage: G.confluent_rewriting_system()
+            Rewriting system of Finitely presented group < a, b | a^2, b^2 >
+            with rules:
+                a^-1    --->    a
+                b^-1    --->    b
+                a^2    --->    1
+                b^2    --->    1
+        """
+        r = RewritingSystem(self)
+        r.make_confluent()
+        return r
+
     from sage.groups.generic import structure_description

src/sage/schemes/curves/zariski_vankampen.py

@@ -1367,8 +1367,8 @@ def braid_monodromy(f, arrangement=(), vertical=False):
     end_braid_computation = False
     while not end_braid_computation:
         try:
-            braidscomputed = (braid_in_segment([(glist, seg[0], seg[1])
-                                                for seg in segs]))
+            braidscomputed = braid_in_segment([(glist, seg[0], seg[1])
+                                               for seg in segs])
             segsbraids = {}
             for braidcomputed in braidscomputed:
                 seg = (braidcomputed[0][0][1], braidcomputed[0][0][2])
@@ -1816,7 +1816,7 @@ def fundamental_group_arrangement(flist, simplified=True, projective=False,
       each of this paths is the conjugated of a loop around one of the points
       in the discriminant of the projection of ``f``.
 
-    - A dictionary attaching to ``j`` a tuple a list of elements
+    - A dictionary attaching to ``j`` a list of elements
       of the group  which are meridians of the curve in position ``j``.
       If ``projective`` is ``False`` and the `y`-degree of the horizontal
       components coincide with the total degree, another key is added
@@ -1925,5 +1925,7 @@ def fundamental_group_arrangement(flist, simplified=True, projective=False,
     n = g1.ngens()
     rels = [rel.Tietze() for rel in g1.relations()]
     g1 = FreeGroup(n) / rels
-    dic1 = {i: list({g1(el.Tietze()) for el in dic1[i]}) for i in dic1}
+    dic1 = {i: [*{t: g1(t) for el in dic1[i] for t in (el.Tietze(),)}.values()] for i in dic1}
+    # each list in dic1.values() may have duplicates, but deduplicating it properly
+    # requires solving the group problem on g1 which can be prohibitive
     return (g1, dic1)

src/sage/topology/simplicial_set_examples.py

@@ -835,25 +835,26 @@ def PresentationComplex(G):
     """
     O = AbstractSimplex(0)
     SO = O.apply_degeneracies(0)
-    edges = {g: AbstractSimplex(1, name=str(g)) for g in G.gens()}
-    inverseedges = {g.inverse(): AbstractSimplex(1, name=str(g.inverse())) for g in G.gens()}
+    F = G.free_group()
+    edges = {g: AbstractSimplex(1, name=str(g)) for g in F.gens()}
+    inverseedges = {g.inverse(): AbstractSimplex(1, name=str(g.inverse())) for g in F.gens()}
     all_edges = {}
     all_edges.update(edges)
     all_edges.update(inverseedges)
-    triangles = {g: AbstractSimplex(2, name='T' + str(g)) for g in G.gens()}
+    triangles = {g: AbstractSimplex(2, name='T' + str(g)) for g in F.gens()}
     face_maps = {g: [O, O] for g in all_edges.values()}
     face_maps.update({triangles[t]: [all_edges[t], SO, all_edges[t.inverse()]] for t in triangles})
     for r in G.relations():
         if len(r.Tietze()) == 1:
             pass
         elif len(r.Tietze()) == 2:
-            a = all_edges[G([r.Tietze()[0]])]
-            b = all_edges[G([r.Tietze()[1]])]
+            a = all_edges[F([r.Tietze()[0]])]
+            b = all_edges[F([r.Tietze()[1]])]
             T = AbstractSimplex(2, name=str(r))
             face_maps[T] = [a, SO, b]
         else:
-            words = [all_edges[G([a])] for a in r.Tietze()]
-            words[-1] = all_edges[G([-r.Tietze()[-1]])]
+            words = [all_edges[F([a])] for a in r.Tietze()]
+            words[-1] = all_edges[F([-r.Tietze()[-1]])]
             while len(words) > 3:
                 auxedge = AbstractSimplex(1)
                 face_maps[auxedge] = [O, O]