From 0b4069c2062358b3c080947d06de158aefc94657 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 01:07:39 -0600
Subject: [PATCH 01/17] Speedup has_right_descent() for Coxeter groups.

---
 src/sage/groups/matrix_gps/coxeter_group.py | 170 ++------------------
 1 file changed, 15 insertions(+), 155 deletions(-)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index d4a46d4933b..666ed5a1e58 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -32,6 +32,7 @@
 from sage.rings.all import ZZ
 from sage.rings.infinity import infinity
 from sage.rings.universal_cyclotomic_field import UniversalCyclotomicField
+from sage.misc.cachefunc import cached_method
 from sage.misc.superseded import deprecated_function_alias
 
 class CoxeterMatrixGroup(FinitelyGeneratedMatrixGroup_generic, UniqueRepresentation):
@@ -274,165 +275,17 @@ def __init__(self, coxeter_matrix, base_ring, index_set):
         FinitelyGeneratedMatrixGroup_generic.__init__(self, ZZ(n), base_ring,
                                                       gens, category=category)
 
-    def _finite_recognition(self):
-        """
-        Return ``True`` if and only if the type is finite.
-
-        This is an auxiliary function used during the initialisation.
-
-        EXAMPLES:
-
-        Some infinite ones::
-
-            sage: F = CoxeterGroups().Finite()
-            sage: W = CoxeterGroup([[1,3,2],[3,1,-1],[2,-1,1]])
-            sage: W in F  # indirect doctest
-            False
-            sage: W = CoxeterGroup([[1,-1,-1],[-1,1,-1],[-1,-1,1]])
-            sage: W in F  # indirect doctest
-            False
-
-        Some finite ones::
-
-            sage: CoxeterGroup(['D',4], base_ring=QQ) in F  # indirect doctest
-            True
-            sage: CoxeterGroup(['H',4]) in F  # indirect doctest
-            True
-        """
-        # First, we build the Coxeter graph of the group, without the
-        # edge labels.
-        coxeter_matrix = self._matrix
-        n = ZZ(coxeter_matrix.nrows())
-        G = Graph([(i, j) for i in range(n) for j in range(i)
-                   if coxeter_matrix[i, j] not in [1, 2]])
-        # Coxeter graphs of finite Coxeter groups are forests
-        if not(G.is_forest()):
-            return False
-        comps = G.connected_components()
-        finite = True
-        # The group is finite if and only if for every connected
-        # component ``comp`` of its Coxeter graph, the submatrix of
-        # the Coxeter matrix corresponding to ``comp`` is one of the
-        # type-A,B,D,E,F,H,I matrices (up to permutation). So we
-        # shall check this condition on every ``comp``.
-        for comp in comps:
-            l = len(comp)
-            if l == 1:
-                # Any `1 \times 1` Coxeter matrix gives a finite group.
-                continue  # A1
-            elif l == 2:
-                # A dihedral group is finite iff there is no `\infty`
-                # in its Coxeter matrix.
-                c0, c1 = comp
-                if coxeter_matrix[c0, c1] > 0:
-                    continue  # I2
-                return False
-            elif l == 3:
-                # The `3`-node case. The finite groups to check for
-                # here are `A_3`, `B_3` and `H_3`.
-                c0, c1, c2 = comp
-                s = sorted([coxeter_matrix[c0, c1],
-                            coxeter_matrix[c0, c2],
-                            coxeter_matrix[c1, c2]])
-                if s[1] == 3 and s[2] in [3, 4, 5]:
-                    continue  # A3, B3, H3
-                return False
-            elif l == 4:
-                # The `4`-node case. The finite groups to check for
-                # here are `A_4`, `B_4`, `D_4`, `F_4` and `H_4`.
-                c0, c1, c2, c3 = comp
-                u = [coxeter_matrix[c0, c1],
-                     coxeter_matrix[c0, c2],
-                     coxeter_matrix[c0, c3],
-                     coxeter_matrix[c1, c2],
-                     coxeter_matrix[c1, c3],
-                     coxeter_matrix[c2, c3]]
-                s = sorted(u)
-                # ``s`` is the list of all off-diagonal entries of
-                # the ``comp``-submatrix of the Coxeter matrix,
-                # sorted in increasing order.
-                if s[3:5] == [3, 3]:
-                    if s[5] == 3:
-                        continue  # A4, D4
-                    if s[5] in [4, 5]:
-                        u0 = u[0] + u[1] + u[2]
-                        u1 = u[0] + u[3] + u[4]
-                        u2 = u[1] + u[3] + u[5]
-                        u3 = u[2] + u[4] + u[5]
-                        ss = sorted([u0, u1, u2, u3])
-                        if ss in [[7, 7, 9, 9], [7, 8, 8, 9],
-                                  [7, 8, 9, 10]]:
-                            continue  # F4, B4, H4
-                return False
-            else:
-                # The case of `l \geq 5` nodes. The finite
-                # groups to check for here are `A_l`, `B_l`, `D_l`,
-                # and `E_l` (for `l = 6, 7, 8`).
-
-                # Checking that the Coxeter matrix of the subgroup
-                # corresponding to the vertices ``comp`` has all its
-                # off-diagonal entries equal to 2, 3 or at most once 4
-                found_a_4 = False
-                for j in range(l):
-                    for i in range(j):
-                        coxeter_entry = coxeter_matrix[comp[i], comp[j]]
-                        if coxeter_entry in [2, 3]:
-                            continue
-                        if coxeter_entry == 4 and not found_a_4:
-                            found_a_4 = True
-                            continue
-                        return False
-
-                G0 = G.subgraph(comp)
-                if found_a_4:
-                    # The case when a `4` has been found in the
-                    # Coxeter matrix. This needs only to be checked
-                    # against `B_l`. We use the observation that
-                    # the group is `B_l` if and only if the Coxeter
-                    # graph is an `l`-path (i.e., has diameter
-                    # `l - 1`) and the `4` corresponds to one of
-                    # its two outermost edges.
-                    diameter = G0.diameter()
-                    if diameter != l - 1:
-                        return False
-
-                    ecc = sorted(((u, v) for (v, u) in G0.eccentricity(with_labels=True).items()))
-                    left_end = ecc[-1][1]
-                    right_end = ecc[-2][1]
-                    left_almost_end = G0.neigbors(left_end)[0]
-                    right_almost_end = G0.neigbors(right_end)[0]
-                    if (coxeter_matrix[left_end, left_almost_end] == 4
-                        or coxeter_matrix[right_end, right_almost_end] == 4):
-                        continue  # Bl
-                    return False
-
-                # Now, all off-diagonal entries of the Coxeter matrix
-                # are 2's and 3's. We need to check our group against
-                # `A_l`, `D_l` and `E_l`. Knowing that the Coxeter
-                # graph is a tree, we can use its vertex
-                # eccentricities to check this.
-                ecc = sorted(G0.eccentricity())
-                if ecc[-1] == l - 1:
-                    continue  # Al
-                if ecc[-3] == l - 2:
-                    continue  # Dl
-                if l <= 8 and ecc[-2] == l - 2 and ecc[-5] == l - 3:
-                    continue  # El
-                return False
-
-        return True
-
     def _repr_(self):
         """
         Return a string representation of ``self``.
 
         EXAMPLES::
 
-            sage: CoxeterGroup([[1,3,2],[3,1,3],[2,3,1]])
+            sage: CoxeterGroup([[1,3,2],[3,1,4],[2,4,1]])
             Finite Coxeter group over Universal Cyclotomic Field with Coxeter matrix:
             [1 3 2]
-            [3 1 3]
-            [2 3 1]
+            [3 1 4]
+            [2 4 1]
         """
         rep = "Finite " if self.is_finite() else ""
         rep += "Coxeter group over {} with Coxeter matrix:\n{}".format(self.base_ring(), self._matrix)
@@ -559,6 +412,7 @@ def is_finite(self):
         # the category of ``self``.
         return "Finite" in self.category().axioms()
 
+    @cached_method
     def order(self):
         """
         Return the order of ``self``.
@@ -575,7 +429,10 @@ def order(self):
             +Infinity
         """
         if self.is_finite():
-            return self._cardinality_from_iterator()
+            try:
+                return ZZ(len(self._list))
+            except AttributeError:
+                return self._cardinality_from_iterator()
         return infinity
 
     def canonical_representation(self):
@@ -646,12 +503,15 @@ def has_right_descent(self, i):
                 sage: W = CoxeterGroup(['A',3], implementation="reflection")
                 sage: a,b,c = W.gens()
                 sage: elt = b*a*c
-                sage: map(lambda i: elt.has_right_descent(i), [1, 2, 3])
+                sage: [elt.has_right_descent(i) for i in [1, 2, 3]]
                 [True, False, True]
             """
             i = self.parent().index_set().index(i)
-            col = self.matrix().column(i)
-            return all(x <= 0 for x in col)
+            n = len(self.parent().index_set())
+            M = self.matrix()
+            # When working over the UCF, this is the bottleneck because it has
+            #   to convert the entries to QQbar and do the comparison there.
+            return all(M[j,i] <= 0 for j in range(n))
 
         def canonical_matrix(self):
             r"""

From c2c3f17434bf907b3601820c495103637473511c Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 01:10:14 -0600
Subject: [PATCH 02/17] Speedup quantum_bruhat_graph by doing some things
 locally.

---
 src/sage/categories/weyl_groups.py | 124 ++++++++++++++++++++---------
 1 file changed, 86 insertions(+), 38 deletions(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index 272085a4b17..170a78fc46f 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -122,15 +122,18 @@ def pieri_factors(self, *args, **keywords):
             raise NotImplementedError("Pieri factors for type {}".format(ct))
 
         @cached_method
-        def quantum_bruhat_graph(self, index_set = ()):
+        def quantum_bruhat_graph(self, index_set=()):
             r"""
-            Returns the quantum Bruhat graph of the quotient of the Weyl group by a parabolic subgroup `W_J`.
+            Return the quantum Bruhat graph of the quotient of the Weyl
+            group by a parabolic subgroup `W_J`.
 
             INPUT:
 
-            - ``index_set`` -- a tuple `J` of nodes of the Dynkin diagram (default: ())
+            - ``index_set`` -- (default: ()) a tuple `J` of nodes of
+              the Dynkin diagram
 
-            By default, the value for ``index_set`` indicates that the subgroup is trivial and the quotient is the full Weyl group.
+            By default, the value for ``index_set`` indicates that the
+            subgroup is trivial and the quotient is the full Weyl group.
 
             EXAMPLES::
 
@@ -141,21 +144,54 @@ def quantum_bruhat_graph(self, index_set = ()):
                 sage: g.vertices()
                 [s2*s3*s1*s2, s3*s1*s2, s1*s2, s3*s2, s2, 1]
                 sage: g.edges()
-                [(s2*s3*s1*s2, s2, alpha[2]), (s3*s1*s2, s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3]),
-                (s3*s1*s2, 1, alpha[2]), (s1*s2, s3*s1*s2, alpha[2] + alpha[3]),
-                (s3*s2, s3*s1*s2, alpha[1] + alpha[2]), (s2, s1*s2, alpha[1] + alpha[2]),
-                (s2, s3*s2, alpha[2] + alpha[3]), (1, s2, alpha[2])]
+                [(s2*s3*s1*s2, s2, alpha[2]),
+                 (s3*s1*s2, s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3]),
+                 (s3*s1*s2, 1, alpha[2]),
+                 (s1*s2, s3*s1*s2, alpha[2] + alpha[3]),
+                 (s3*s2, s3*s1*s2, alpha[1] + alpha[2]),
+                 (s2, s1*s2, alpha[1] + alpha[2]),
+                 (s2, s3*s2, alpha[2] + alpha[3]),
+                 (1, s2, alpha[2])]
                 sage: W = WeylGroup(['A',3,1], prefix="s")
                 sage: g = W.quantum_bruhat_graph()
                 Traceback (most recent call last):
                 ...
-                ValueError: The Cartan type ['A', 3, 1] is not finite
+                ValueError: the Cartan type ['A', 3, 1] is not finite
             """
             if not self.cartan_type().is_finite():
-                raise ValueError("The Cartan type {} is not finite".format(self.cartan_type()))
+                raise ValueError("the Cartan type {} is not finite".format(self.cartan_type()))
             from sage.graphs.digraph import DiGraph
-            WP = [x for x in self if x==x.coset_representative(index_set)]
-            return DiGraph([[x,i[0],i[1]] for x in WP for i in x.quantum_bruhat_successors(index_set, roots = True)],
+            WP = [x for x in self if x == x.coset_representative(index_set)]
+
+            # This is a modified form of quantum_bruhat_successors.
+            # It does not do any error checking and also is more efficient
+            #   with how it handles memory and checks by using data stored
+            #   at this function level rather than recomputing everything.
+            lattice = self.cartan_type().root_system().root_lattice()
+            NPR = lattice.nonparabolic_positive_roots(index_set)
+            NPR_sum = sum(NPR)
+            NPR_data = {}
+            full_NPR_sum = lattice.nonparabolic_positive_root_sum(())
+            for alpha in NPR:
+                ref = alpha.associated_reflection()
+                alphacheck = alpha.associated_coroot()
+                NPR_data[alpha] = [self.from_reduced_word(ref), # the element
+                                   len(ref) == full_NPR_sum.scalar(alphacheck) - 1, # is_quantum
+                                   NPR_sum.scalar(alphacheck)] # the scalar
+            def succ(x):
+                w_length_plus_one = x.length() + 1
+                successors = []
+                for alpha in NPR:
+                    elt, is_quantum, scalar = NPR_data[alpha]
+                    wr = x * elt
+                    wrc = wr.coset_representative(index_set)
+                    if wrc == wr and wr.length() == w_length_plus_one:
+                        successors.append((wr, alpha))
+                    elif is_quantum and wrc.length() == w_length_plus_one - scalar:
+                        successors.append((wrc, alpha))
+                return successors
+
+            return DiGraph([[x,i[0],i[1]] for x in WP for i in succ(x)],
                            name="Parabolic Quantum Bruhat Graph of %s for nodes %s"%(self, index_set))
 
     class ElementMethods:
@@ -560,24 +596,26 @@ def inversion_arrangement(self, side='right'):
 
         def bruhat_lower_covers_coroots(self):
             r"""
-            Returns all 2-tuples (``v``, `\alpha`) where ``v`` is covered by ``self`` and `\alpha`
-            is the positive coroot such that ``self`` = ``v`` `s_\alpha` where `s_\alpha` is
+            Return all 2-tuples (``v``, `\alpha`) where ``v`` is covered
+            by ``self`` and `\alpha` is the positive coroot such that
+            ``self`` = ``v`` `s_\alpha` where `s_\alpha` is
             the reflection orthogonal to `\alpha`.
 
             ALGORITHM:
 
-            See :meth:`.bruhat_lower_covers` and :meth:`.bruhat_lower_covers_reflections` for Coxeter groups.
+            See :meth:`.bruhat_lower_covers` and
+            :meth:`.bruhat_lower_covers_reflections` for Coxeter groups.
 
             EXAMPLES::
 
                 sage: W = WeylGroup(['A',3], prefix="s")
                 sage: w = W.from_reduced_word([3,1,2,1])
                 sage: w.bruhat_lower_covers_coroots()
-                [(s1*s2*s1, alphacheck[1] + alphacheck[2] + alphacheck[3]), (s3*s2*s1, alphacheck[2]), (s3*s1*s2, alphacheck[1])]
-
+                [(s1*s2*s1, alphacheck[1] + alphacheck[2] + alphacheck[3]),
+                 (s3*s2*s1, alphacheck[2]), (s3*s1*s2, alphacheck[1])]
             """
-
-            return [(x[0],x[1].reflection_to_coroot()) for x in self.bruhat_lower_covers_reflections()]
+            return [(x[0],x[1].reflection_to_coroot())
+                    for x in self.bruhat_lower_covers_reflections()]
 
         def bruhat_upper_covers_coroots(self):
             r"""
@@ -594,41 +632,52 @@ def bruhat_upper_covers_coroots(self):
                 sage: W = WeylGroup(['A',4], prefix="s")
                 sage: w = W.from_reduced_word([3,1,2,1])
                 sage: w.bruhat_upper_covers_coroots()
-                [(s1*s2*s3*s2*s1, alphacheck[3]), (s2*s3*s1*s2*s1, alphacheck[2] + alphacheck[3]), (s3*s4*s1*s2*s1, alphacheck[4]), (s4*s3*s1*s2*s1, alphacheck[1] + alphacheck[2] + alphacheck[3] + alphacheck[4])]
-
+                [(s1*s2*s3*s2*s1, alphacheck[3]),
+                 (s2*s3*s1*s2*s1, alphacheck[2] + alphacheck[3]),
+                 (s3*s4*s1*s2*s1, alphacheck[4]),
+                 (s4*s3*s1*s2*s1, alphacheck[1] + alphacheck[2] + alphacheck[3] + alphacheck[4])]
             """
+            return [(x[0],x[1].reflection_to_coroot())
+                    for x in self.bruhat_upper_covers_reflections()]
 
-            return [(x[0],x[1].reflection_to_coroot()) for x in self.bruhat_upper_covers_reflections()]
-
-        def quantum_bruhat_successors(self, index_set = None, roots = False, quantum_only = False):
+        def quantum_bruhat_successors(self, index_set=None, roots=False, quantum_only=False):
             r"""
-            Returns the successors of ``self`` in the parabolic quantum Bruhat graph.
+            Return the successors of ``self`` in the quantum Bruhat graph
+            on the parabolic quotient of the Weyl group determined by the
+            subset of Dynkin nodes ``index_set``.
 
             INPUT:
 
-            - ``self`` -- a Weyl group element, which is assumed to be of minimum length in its coset with respect to the parabolic subgroup
-
-            - ``index_set`` -- (default: None) indicates the set of simple reflections used to generate the parabolic subgroup;
-               the default value indicates that the subgroup is the identity
+            - ``self`` -- a Weyl group element, which is assumed to
+              be of minimum length in its coset with respect to the
+              parabolic subgroup
 
-            - ``roots`` -- (default: False) if True, returns the list of 2-tuples (``w``, `\alpha`) where ``w`` is a
-               successor and `\alpha` is the positive root associated with the successor relation.
+            - ``index_set`` -- (default: ``None``) indicates the set of
+              simple reflections used to generate the parabolic subgroup;
+              the default value indicates that the subgroup is the identity
 
-            - ``quantum_only`` -- (default: False) if True, returns only the quantum successors
+            - ``roots`` -- (default: ``False``) if ``True``, returns the
+              list of 2-tuples (``w``, `\alpha`) where ``w`` is a successor
+              and `\alpha` is the positive root associated with the
+              successor relation
 
-            Returns the successors of ``self`` in the quantum Bruhat graph on the parabolic
-            quotient of the Weyl group determined by the subset of Dynkin nodes ``index_set``.
+            - ``quantum_only`` -- (default: ``False``) if ``True``, returns
+              only the quantum successors
 
             EXAMPLES::
 
                 sage: W = WeylGroup(['A',3], prefix="s")
                 sage: w = W.from_reduced_word([3,1,2])
                 sage: w.quantum_bruhat_successors([1], roots = True)
-                [(s3, alpha[2]), (s1*s2*s3*s2, alpha[3]), (s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3])]
+                [(s3, alpha[2]), (s1*s2*s3*s2, alpha[3]),
+                 (s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3])]
                 sage: w.quantum_bruhat_successors([1,3])
                 [1, s2*s3*s1*s2]
                 sage: w.quantum_bruhat_successors(roots = True)
-                [(s3*s1*s2*s1, alpha[1]), (s3*s1, alpha[2]), (s1*s2*s3*s2, alpha[3]), (s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3])]
+                [(s3*s1*s2*s1, alpha[1]),
+                 (s3*s1, alpha[2]),
+                 (s1*s2*s3*s2, alpha[3]),
+                 (s2*s3*s1*s2, alpha[1] + alpha[2] + alpha[3])]
                 sage: w.quantum_bruhat_successors()
                 [s3*s1*s2*s1, s3*s1, s1*s2*s3*s2, s2*s3*s1*s2]
                 sage: w.quantum_bruhat_successors(quantum_only = True)
@@ -638,11 +687,10 @@ def quantum_bruhat_successors(self, index_set = None, roots = False, quantum_onl
                 Traceback (most recent call last):
                 ...
                 ValueError: s2*s3 is not of minimum length in its coset of the parabolic subgroup generated by the reflections (1, 3)
-
             """
             W = self.parent()
             if not W.cartan_type().is_finite():
-                raise ValueError("The Cartan type {} is not finite".format(W.cartan_type()))
+                raise ValueError("the Cartan type {} is not finite".format(W.cartan_type()))
             if index_set is None:
                 index_set = []
             else:

From 2b1a1175b41c07d2971fb626556bec32b9b55308 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 02:53:17 -0600
Subject: [PATCH 03/17] Changed libs.gap.element.GapElement.matrix() to avoid
 matrix constructor.

---
 src/sage/libs/gap/element.pyx | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/sage/libs/gap/element.pyx b/src/sage/libs/gap/element.pyx
index a9a39ad224e..ffc3c7c6b53 100644
--- a/src/sage/libs/gap/element.pyx
+++ b/src/sage/libs/gap/element.pyx
@@ -1098,10 +1098,11 @@ cdef class GapElement(RingElement):
         m = len(entries) // n
         if len(entries) % n != 0:
             raise ValueError('not a rectangular list of lists')
-        from sage.matrix.constructor import matrix
+        from sage.matrix.matrix_space import MatrixSpace
         if ring is None:
             ring = entries.DefaultRing().sage()
-        return matrix(ring, n, m, [ x.sage(ring=ring) for x in entries ])
+        MS = MatrixSpace(ring, n, m)
+        return MS([x.sage(ring=ring) for x in entries])
 
     _matrix_ = matrix
 

From 24261553c6f3c2b9496f89ff09a2d404be4e8750 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 02:54:51 -0600
Subject: [PATCH 04/17] Use a specialized version of GapElement.matrix() to
 avoid some overhead.

---
 src/sage/groups/matrix_gps/group_element.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/sage/groups/matrix_gps/group_element.py b/src/sage/groups/matrix_gps/group_element.py
index 434c7ce8027..e84d78d44ca 100644
--- a/src/sage/groups/matrix_gps/group_element.py
+++ b/src/sage/groups/matrix_gps/group_element.py
@@ -433,8 +433,13 @@ def matrix(self):
             sage: _.parent()
             Full MatrixSpace of 2 by 2 dense matrices over Finite Field of size 3
         """
-        g = self.gap()
-        m = g.matrix(self.base_ring())
+        # We do a slightly specialized version of sage.libs.gap.element.GapElement.matrix()
+        #   in order to use our current matrix space directly and avoid
+        #   some overhead safety checks.
+        entries = self.gap().Flat()
+        MS = self.parent().matrix_space()
+        ring = MS.base_ring()
+        m = MS([x.sage(ring=ring) for x in entries])
         m.set_immutable()
         return m
 

From 4caa2bfe80f22253fdc960ec7e85c73649ebc41e Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 02:59:28 -0600
Subject: [PATCH 05/17] Don't store matrix() of Weyl group elements and some
 cleanup.

We don't always need to compute the matrix. Moreover, because
the result of matrix() is cached, it is on par with the
(python) attribute lookup.

Also added a has_right_descent() to avoid a possible (double)
inverting of the element if called.
---
 src/sage/combinat/root_system/weyl_group.py | 54 +++++++++++++++------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/sage/combinat/root_system/weyl_group.py b/src/sage/combinat/root_system/weyl_group.py
index 1efef94c3bd..e85a58600f5 100644
--- a/src/sage/combinat/root_system/weyl_group.py
+++ b/src/sage/combinat/root_system/weyl_group.py
@@ -704,11 +704,10 @@ def __init__(self, parent, g, check=False):
             sage: TestSuite(s1).run()
         """
         MatrixGroupElement_gap.__init__(self, parent, g, check=check)
-        self.__matrix = self.matrix()
         self._parent = parent
 
     def __hash__(self):
-        return hash(self.__matrix)
+        return hash(self.matrix())
 
     def domain(self):
         """
@@ -796,7 +795,7 @@ def __eq__(self, other):
         """
         return self.__class__ == other.__class__ and \
                self._parent   == other._parent   and \
-               self.__matrix  == other.__matrix
+               self.matrix()  == other.matrix()
 
     def _cmp_(self, other):
         """
@@ -843,7 +842,7 @@ def action(self, v):
         """
         if v not in self.domain():
             raise ValueError("{} is not in the domain".format(v))
-        return self.domain().from_vector(self.__matrix*v.to_vector())
+        return self.domain().from_vector(self.matrix()*v.to_vector())
 
 
     ##########################################################################
@@ -852,12 +851,14 @@ def action(self, v):
 
     def has_descent(self, i, positive=False, side = "right"):
         """
-        Tests if self has a descent at position `i`, that is if self is
+        Test if ``self`` has a descent at position ``i``.
+
+        An element `w` has a descent in position `i` if `w` is
         on the strict negative side of the `i^{th}` simple reflection
         hyperplane.
 
-        If positive is True, tests if it is on the strict positive
-        side instead.
+        If ``positive`` is ``True``, tests if it is on the strict
+        positive side instead.
 
         EXAMPLES::
 
@@ -914,33 +915,54 @@ def has_descent(self, i, positive=False, side = "right"):
             self = ~self
 
         if use_rho:
-            s = self.action(L.rho()   ).scalar(L.alphacheck()[i]) >= 0
+            s = self.action(L.rho()).scalar(L.alphacheck()[i]) >= 0
         else:
             s = self.action(L.alpha()[i]).is_positive_root()
 
         return s is positive
 
-    def has_left_descent(self,i):
+    def has_left_descent(self, i):
         """
-        Tests if self has a left descent at position `i`.
+        Test if ``self`` has a left descent at position ``i``.
 
         EXAMPLES::
 
             sage: W = WeylGroup(['A',3])
             sage: s = W.simple_reflections()
-            sage: [W.one().has_descent(i) for i in W.domain().index_set()]
+            sage: [W.one().has_left_descent(i) for i in W.domain().index_set()]
             [False, False, False]
-            sage: [s[1].has_descent(i) for i in W.domain().index_set()]
+            sage: [s[1].has_left_descent(i) for i in W.domain().index_set()]
             [True, False, False]
-            sage: [s[2].has_descent(i) for i in W.domain().index_set()]
+            sage: [s[2].has_left_descent(i) for i in W.domain().index_set()]
             [False, True, False]
-            sage: [s[3].has_descent(i) for i in W.domain().index_set()]
+            sage: [s[3].has_left_descent(i) for i in W.domain().index_set()]
+            [False, False, True]
+            sage: [(s[3]*s[2]).has_left_descent(i) for i in W.domain().index_set()]
             [False, False, True]
-            sage: [s[3].has_descent(i, True) for i in W.domain().index_set()]
-            [True, True, False]
         """
         return self.has_descent(i, side = "left")
 
+    def has_right_descent(self, i):
+        """
+        Test if ``self`` has a right descent at position ``i``.
+
+        EXAMPLES::
+
+            sage: W = WeylGroup(['A',3])
+            sage: s = W.simple_reflections()
+            sage: [W.one().has_right_descent(i) for i in W.domain().index_set()]
+            [False, False, False]
+            sage: [s[1].has_right_descent(i) for i in W.domain().index_set()]
+            [True, False, False]
+            sage: [s[2].has_right_descent(i) for i in W.domain().index_set()]
+            [False, True, False]
+            sage: [s[3].has_right_descent(i) for i in W.domain().index_set()]
+            [False, False, True]
+            sage: [(s[3]*s[2]).has_right_descent(i) for i in W.domain().index_set()]
+            [False, True, False]
+        """
+        return self.has_descent(i, side="right")
+
     def apply_simple_reflection(self, i, side = "right"):
         s = self.parent().simple_reflections()
         if side == "right":

From fcf0e350613f1c954dacb68881631f42ba06c588 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 03:01:38 -0600
Subject: [PATCH 06/17] Added length cache to quantum_bruhat_graph().

---
 src/sage/categories/weyl_groups.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index 170a78fc46f..06faa20afa9 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -178,16 +178,24 @@ def quantum_bruhat_graph(self, index_set=()):
                 NPR_data[alpha] = [self.from_reduced_word(ref), # the element
                                    len(ref) == full_NPR_sum.scalar(alphacheck) - 1, # is_quantum
                                    NPR_sum.scalar(alphacheck)] # the scalar
+            # We also create a temporary cache of lengths as they are
+            #   relatively expensive to compute and needed frequently
+            len_cache = {}
+            def length(x):
+                if x in len_cache:
+                    return len_cache[x]
+                len_cache[x] = x.length()
+                return len_cache[x]
             def succ(x):
-                w_length_plus_one = x.length() + 1
+                w_length_plus_one = length(x) + 1
                 successors = []
                 for alpha in NPR:
                     elt, is_quantum, scalar = NPR_data[alpha]
                     wr = x * elt
                     wrc = wr.coset_representative(index_set)
-                    if wrc == wr and wr.length() == w_length_plus_one:
+                    if wrc == wr and length(wr) == w_length_plus_one:
                         successors.append((wr, alpha))
-                    elif is_quantum and wrc.length() == w_length_plus_one - scalar:
+                    elif is_quantum and length(wrc) == w_length_plus_one - scalar:
                         successors.append((wrc, alpha))
                 return successors
 

From 0b5fcec3ea079b5240414f4981bd860d7768b3c7 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 03:15:59 -0600
Subject: [PATCH 07/17] Get a little more speed by using the matrices as keys
 for the length cache.

---
 src/sage/categories/weyl_groups.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index 06faa20afa9..ba953c05376 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -182,10 +182,12 @@ def quantum_bruhat_graph(self, index_set=()):
             #   relatively expensive to compute and needed frequently
             len_cache = {}
             def length(x):
-                if x in len_cache:
-                    return len_cache[x]
-                len_cache[x] = x.length()
-                return len_cache[x]
+                # It is sufficient and faster to use the matrices as the keys
+                m = x.matrix()
+                if m in len_cache:
+                    return len_cache[m]
+                len_cache[m] = x.length()
+                return len_cache[m]
             def succ(x):
                 w_length_plus_one = length(x) + 1
                 successors = []

From f673aec39c5dbb81c65ae72fd70154c5674585f5 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 04:20:53 -0600
Subject: [PATCH 08/17] A better method to find the minimal length coset
 representatives.

---
 src/sage/categories/weyl_groups.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index ba953c05376..a2b05886c72 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -160,8 +160,9 @@ def quantum_bruhat_graph(self, index_set=()):
             """
             if not self.cartan_type().is_finite():
                 raise ValueError("the Cartan type {} is not finite".format(self.cartan_type()))
-            from sage.graphs.digraph import DiGraph
-            WP = [x for x in self if x == x.coset_representative(index_set)]
+
+            # Find all the minimal length coset representatives
+            WP = [x for x in self if all(not x.has_descent(i) for i in index_set)]
 
             # This is a modified form of quantum_bruhat_successors.
             # It does not do any error checking and also is more efficient
@@ -201,6 +202,7 @@ def succ(x):
                         successors.append((wrc, alpha))
                 return successors
 
+            from sage.graphs.digraph import DiGraph
             return DiGraph([[x,i[0],i[1]] for x in WP for i in succ(x)],
                            name="Parabolic Quantum Bruhat Graph of %s for nodes %s"%(self, index_set))
 

From af33acf5e347a5fa0aec66025f3837ea728061ce Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 17:30:43 -0600
Subject: [PATCH 09/17] Added explicit format indicator for DiGraph constructor
 for Darij.

---
 src/sage/categories/weyl_groups.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index a2b05886c72..1902fb3ae88 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -204,7 +204,8 @@ def succ(x):
 
             from sage.graphs.digraph import DiGraph
             return DiGraph([[x,i[0],i[1]] for x in WP for i in succ(x)],
-                           name="Parabolic Quantum Bruhat Graph of %s for nodes %s"%(self, index_set))
+                           name="Parabolic Quantum Bruhat Graph of %s for nodes %s"%(self, index_set),
+                           format="list_of_edges")
 
     class ElementMethods:
 

From e0140371806e13560a59869fd540ef8cbf6e8a8b Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 18:33:44 -0600
Subject: [PATCH 10/17] Making some additional improvements.

---
 src/sage/groups/matrix_gps/coxeter_group.py |  3 ++-
 src/sage/groups/matrix_gps/group_element.py | 11 ++++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index 666ed5a1e58..d24aba9aa44 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -509,9 +509,10 @@ def has_right_descent(self, i):
             i = self.parent().index_set().index(i)
             n = len(self.parent().index_set())
             M = self.matrix()
+            zero = M.base_ring().zero()
             # When working over the UCF, this is the bottleneck because it has
             #   to convert the entries to QQbar and do the comparison there.
-            return all(M[j,i] <= 0 for j in range(n))
+            return all(M[j,i] <= zero for j in range(n))
 
         def canonical_matrix(self):
             r"""
diff --git a/src/sage/groups/matrix_gps/group_element.py b/src/sage/groups/matrix_gps/group_element.py
index e84d78d44ca..61d0eae0271 100644
--- a/src/sage/groups/matrix_gps/group_element.py
+++ b/src/sage/groups/matrix_gps/group_element.py
@@ -322,8 +322,10 @@ def _mul_(self,other):
             [0 1]
         """
         parent = self.parent()
-        return parent.element_class(parent, self._matrix * other._matrix,
-                                    check=False, convert=False)
+        M = self._matrix * other._matrix
+        # Make it immutable so the constructor doesn't make a copy
+        M.set_immutable()
+        return parent.element_class(parent, M, check=False, convert=False)
 
     def __invert__(self):
         """
@@ -347,7 +349,10 @@ def __invert__(self):
            [0 1]
         """
         parent = self.parent()
-        return parent.element_class(parent, ~self._matrix, check=False, convert=False)
+        M = ~self._matrix
+        # Make it immutable so the constructor doesn't make a copy
+        M.set_immutable()
+        return parent.element_class(parent, M, check=False, convert=False)
 
     inverse = __invert__
 

From 1129ab7725b584d019902863ae6949d6996fdbc6 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sat, 2 Jan 2016 18:38:03 -0600
Subject: [PATCH 11/17] Removing comment about bottlenecks.

---
 src/sage/groups/matrix_gps/coxeter_group.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index d24aba9aa44..55e961c8c53 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -510,8 +510,6 @@ def has_right_descent(self, i):
             n = len(self.parent().index_set())
             M = self.matrix()
             zero = M.base_ring().zero()
-            # When working over the UCF, this is the bottleneck because it has
-            #   to convert the entries to QQbar and do the comparison there.
             return all(M[j,i] <= zero for j in range(n))
 
         def canonical_matrix(self):

From 8509534a8900501aca69724f27920c064dc7e618 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sun, 3 Jan 2016 05:03:46 -0600
Subject: [PATCH 12/17] Implementing custon first_descent() and descent()
 methods for CoxeterGroup.

---
 src/sage/groups/matrix_gps/coxeter_group.py | 69 +++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index 55e961c8c53..00b0605aa86 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -480,6 +480,75 @@ class Element(MatrixGroupElement_generic):
         """
         A Coxeter group element.
         """
+        def first_descent(self, side = 'right', index_set=None, positive=False):
+            """
+            Return the first left (resp. right) descent of ``self``, as
+            ane element of ``index_set``, or ``None`` if there is none.
+
+            See :meth:`descents` for a description of the options.
+
+            EXAMPLES::
+            """
+            M = self.matrix()
+            if side != 'right':
+                M = ~M
+            I = self.parent().index_set()
+            n = len(I)
+            zero = M.base_ring().zero()
+            if index_set is None:
+                index_set = range(n)
+            else:
+                index_set = [I.index(i) for i in index_set]
+            if positive:
+                for i in index_set:
+                    if any(M[j,i] > zero for j in range(n)):
+                        return I[i]
+            else:
+                for i in index_set:
+                    if all(M[j,i] <= zero for j in range(n)):
+                        return I[i]
+            return None
+
+        def descents(self, side='right', index_set=None, positive=False):
+            """
+            Return the descents of ``self``, as a list of elements of the
+            ``index_set``.
+
+            INPUT:
+
+            - ``index_set`` -- (default: all of them) a subset (as a list
+              or iterable) of the nodes of the Dynkin diagram
+            - ``side`` -- (default: ``'right'``) ``'left'`` or ``'right'``
+            - ``positive`` -- (default: ``False``) boolean
+
+            EXAMPLES::
+
+                sage: W = CoxeterGroup(['A',3], implementation="reflection")
+                sage: a,b,c = W.gens()
+                sage: elt = b*a*c
+                sage: elt.descents()
+                [1, 3]
+                sage: elt.descents(positive=True)
+                [2]
+                sage: elt.descents(index_set=[1,2])
+                [1]
+                sage: elt.descents(side='left')
+                [2]
+            """
+            M = self.matrix()
+            if side != 'right':
+                M = ~M
+            I = self.parent().index_set()
+            n = len(I)
+            zero = M.base_ring().zero()
+            if index_set is None:
+                index_set = range(n)
+            else:
+                index_set = [I.index(i) for i in index_set]
+            if positive:
+                return [I[i] for i in index_set if any(M[j,i] > zero for j in range(n))]
+            return [I[i] for i in index_set if all(M[j,i] <= zero for j in range(n))]
+
         def has_right_descent(self, i):
             r"""
             Return whether ``i`` is a right descent of ``self``.

From 2fdaef455cf27e35c207ef217353e974e25288ae Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Sun, 3 Jan 2016 17:52:31 -0600
Subject: [PATCH 13/17] Forgot the doctests for first_descent.

---
 src/sage/groups/matrix_gps/coxeter_group.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index 00b0605aa86..9bce929fd59 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -488,6 +488,14 @@ def first_descent(self, side = 'right', index_set=None, positive=False):
             See :meth:`descents` for a description of the options.
 
             EXAMPLES::
+
+                sage: W = CoxeterGroup(['A',3], implementation="reflection")
+                sage: a,b,c = W.gens()
+                sage: elt = b*a*c
+                sage: elt.first_descent()
+                1
+                sage: elt.first_descent(side='left')
+                2
             """
             M = self.matrix()
             if side != 'right':

From 2e9535ad09e6aa2489848f51f95452d2cbfd5884 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Thu, 21 Jan 2016 11:19:14 -0600
Subject: [PATCH 14/17] Make the generators of CoxeterGroup dense.

The one() element is dense because the matrix_space() is
dense. By making the generators dense, it means the generators
are not converted to dense matrices upon multiplication,
resulting in a 2x speedup to iteration.
---
 src/sage/groups/matrix_gps/coxeter_group.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/sage/groups/matrix_gps/coxeter_group.py b/src/sage/groups/matrix_gps/coxeter_group.py
index f9a68e9983b..037e19a8223 100644
--- a/src/sage/groups/matrix_gps/coxeter_group.py
+++ b/src/sage/groups/matrix_gps/coxeter_group.py
@@ -266,6 +266,8 @@ def __init__(self, coxeter_matrix, base_ring, index_set):
                                            for j in range(n)},
                               coerce=True, copy=True)
                 for i in range(n)]
+        # Make the generators dense matrices for consistancy and speed
+        gens = [g.dense_matrix() for g in gens]
         category = CoxeterGroups()
         # Now we shall see if the group is finite, and, if so, refine
         # the category to ``category.Finite()``. Otherwise the group is

From 9250e99268d90500411c21df4a6afed78865ccb3 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Tue, 8 Mar 2016 17:49:52 -0600
Subject: [PATCH 15/17] Fixing doctest and adding is_one.

---
 src/sage/groups/matrix_gps/group_element.pxd |  1 +
 src/sage/groups/matrix_gps/group_element.pyx | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/sage/groups/matrix_gps/group_element.pxd b/src/sage/groups/matrix_gps/group_element.pxd
index eba68df1bea..ae35220720e 100644
--- a/src/sage/groups/matrix_gps/group_element.pxd
+++ b/src/sage/groups/matrix_gps/group_element.pxd
@@ -10,6 +10,7 @@ cdef class MatrixGroupElement_generic(MultiplicativeGroupElement):
     cpdef int _cmp_(self, Element other) except -2
     cpdef list list(self)
     cpdef MonoidElement _mul_(self, MonoidElement other)
+    cpdef is_one(self)
 
 cdef class MatrixGroupElement_gap(ElementLibGAP):
     cpdef _act_on_(self, x, bint self_on_left)
diff --git a/src/sage/groups/matrix_gps/group_element.pyx b/src/sage/groups/matrix_gps/group_element.pyx
index 941ec100d12..f00db1b72dc 100644
--- a/src/sage/groups/matrix_gps/group_element.pyx
+++ b/src/sage/groups/matrix_gps/group_element.pyx
@@ -291,7 +291,7 @@ cdef class MatrixGroupElement_generic(MultiplicativeGroupElement):
             [ 0  1  0]
             [ 0  0  1]
             sage: parent(g.matrix())
-            Full MatrixSpace of 3 by 3 sparse matrices over Integer Ring
+            Full MatrixSpace of 3 by 3 dense matrices over Integer Ring
 
         Matrices have extra functionality that matrix group elements
         do not have::
@@ -323,6 +323,24 @@ cdef class MatrixGroupElement_generic(MultiplicativeGroupElement):
         M.set_immutable()
         return parent.element_class(parent, M, check=False, convert=False)
 
+    cpdef is_one(self):
+        """
+        Return whether ``self`` is the identity of the group.
+
+        EXAMPLES::
+
+            sage: W = CoxeterGroup(['A',3])
+            sage: g = W.gen(0)
+            sage: g.is_one()
+            False
+
+            sage: W.an_element().is_one()
+            False
+            sage: W.one().is_one()
+            True
+        """
+        return self._matrix.is_one()
+
     def __invert__(self):
         """
         Return the inverse group element

From bea6b3c96b42255a8f9459b344c625fe22c337d7 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Tue, 8 Mar 2016 18:00:05 -0600
Subject: [PATCH 16/17] Even better is_one by avoiding unnecessary coercions
 for matrices.

---
 src/sage/groups/matrix_gps/group_element.pxd | 1 -
 src/sage/groups/matrix_gps/group_element.pyx | 2 +-
 src/sage/matrix/matrix2.pyx                  | 4 ++--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/sage/groups/matrix_gps/group_element.pxd b/src/sage/groups/matrix_gps/group_element.pxd
index ae35220720e..eba68df1bea 100644
--- a/src/sage/groups/matrix_gps/group_element.pxd
+++ b/src/sage/groups/matrix_gps/group_element.pxd
@@ -10,7 +10,6 @@ cdef class MatrixGroupElement_generic(MultiplicativeGroupElement):
     cpdef int _cmp_(self, Element other) except -2
     cpdef list list(self)
     cpdef MonoidElement _mul_(self, MonoidElement other)
-    cpdef is_one(self)
 
 cdef class MatrixGroupElement_gap(ElementLibGAP):
     cpdef _act_on_(self, x, bint self_on_left)
diff --git a/src/sage/groups/matrix_gps/group_element.pyx b/src/sage/groups/matrix_gps/group_element.pyx
index f00db1b72dc..23610df1444 100644
--- a/src/sage/groups/matrix_gps/group_element.pyx
+++ b/src/sage/groups/matrix_gps/group_element.pyx
@@ -323,7 +323,7 @@ cdef class MatrixGroupElement_generic(MultiplicativeGroupElement):
         M.set_immutable()
         return parent.element_class(parent, M, check=False, convert=False)
 
-    cpdef is_one(self):
+    def is_one(self):
         """
         Return whether ``self`` is the identity of the group.
 
diff --git a/src/sage/matrix/matrix2.pyx b/src/sage/matrix/matrix2.pyx
index 7a8fab5da83..fe7ad13e4d5 100644
--- a/src/sage/matrix/matrix2.pyx
+++ b/src/sage/matrix/matrix2.pyx
@@ -8124,7 +8124,7 @@ explicitly setting the argument to `True` or `False` will avoid this message."""
             sage: m.is_one()
             False
         """
-        return self.is_scalar(1)
+        return self.is_scalar(self.base_ring().one())
 
     def is_scalar(self, a = None):
         """
@@ -8164,7 +8164,7 @@ explicitly setting the argument to `True` or `False` will avoid this message."""
             a = self.get_unsafe(0,0)
         else:
             a = self.base_ring()(a)
-        zero = self.base_ring()(0)
+        zero = self.base_ring().zero()
         for i from 0 <= i < self._nrows:
             for j from 0 <= j < self._ncols:
                 if i != j:

From f38620abeb1d42b0a289c19e034c169091e80f18 Mon Sep 17 00:00:00 2001
From: Travis Scrimshaw <tscrimsh at umn.edu>
Date: Tue, 8 Mar 2016 19:50:08 -0600
Subject: [PATCH 17/17] Check identity rather than equality from
 coset_representative.

---
 src/sage/categories/weyl_groups.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/sage/categories/weyl_groups.py b/src/sage/categories/weyl_groups.py
index 1902fb3ae88..26884bb3a4b 100644
--- a/src/sage/categories/weyl_groups.py
+++ b/src/sage/categories/weyl_groups.py
@@ -196,7 +196,8 @@ def succ(x):
                     elt, is_quantum, scalar = NPR_data[alpha]
                     wr = x * elt
                     wrc = wr.coset_representative(index_set)
-                    if wrc == wr and length(wr) == w_length_plus_one:
+                    # coset_representative returns wr if nothing gets changed
+                    if wrc is wr and length(wr) == w_length_plus_one:
                         successors.append((wr, alpha))
                     elif is_quantum and length(wrc) == w_length_plus_one - scalar:
                         successors.append((wrc, alpha))