Sage: Ticket #15456: fix bug in has_right/left_descents in Weyl group code
https://trac.sagemath.org/ticket/15456
<p>
The <code>has_right_descents</code> method in <code>WeylGroup</code> element methods calls <code>has_left_descents</code> and vice versa. This causes an error in the following example.
</p>
<pre class="wiki">sage: W = WeylGroup(['A',4])
sage: w = W.from_reduced_word([3,4,2])
sage: w.has_right_descent(3)
Traceback (click to the left of this block for traceback)
...
RuntimeError: maximum recursion depth exceeded
</pre><p>
The doc tests for this method make calls to <code>CoxeterGroup</code> code to test it and so all tests pass.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/15456
Trac 1.1.6nthieryTue, 26 Nov 2013 22:20:06 GMT
https://trac.sagemath.org/ticket/15456#comment:1
https://trac.sagemath.org/ticket/15456#comment:1
<blockquote>
<p>
Hi Mike!
</p>
</blockquote>
<p>
This is not a bug but a feature :-)
</p>
<p>
When implementing a Coxeter group one can choose to either implement has_right_descent or has_left_descent (or both), and the Coxeter group category provides a default implementation for the other, if needed.
</p>
<p>
Granted, it would be nice if there was a nicer error message mentionning that at least one of has_left_descent or has_right_descent should be implemented. However, at this point, we have not generic infrastructure for this kind of situation, and it would be a bit tedious to do it by hand (but ideas on how to handle this are welcome!).
</p>
<p>
Cheers,
</p>
TicketzabrockiTue, 26 Nov 2013 22:25:38 GMT
https://trac.sagemath.org/ticket/15456#comment:2
https://trac.sagemath.org/ticket/15456#comment:2
<p>
Hi Nicolas,
</p>
<p>
This can't be a feature. In <code>WeylGroup</code> this is a bug. In <code>CoxeterGroup</code> it is correct.
</p>
<p>
In <code>WeylGroups</code> the method <code>has_right_descent</code> and <code>has_left_descent</code> don't do anything except call each other.
</p>
TicketzabrockiTue, 26 Nov 2013 22:27:45 GMT
https://trac.sagemath.org/ticket/15456#comment:3
https://trac.sagemath.org/ticket/15456#comment:3
<p>
BTW, in <code>WeylGroup</code> the method <code>has_descent</code> doesn't call either <code>has_right_descent</code> or <code>has_left_descent</code> and decides if there is a right/left descent by another method.
</p>
TicketnthieryTue, 26 Nov 2013 22:52:05 GMT
https://trac.sagemath.org/ticket/15456#comment:4
https://trac.sagemath.org/ticket/15456#comment:4
<p>
Right. The proper specification is:
</p>
<ul><li>at least one of has_descent / has_right_descent / has_left_descent should be implemented
</li><li>in generic code, always use has_descent, not has_*_descent.
</li></ul><p>
This definitely should be put in the doc if not yet there.
</p>
TicketzabrockiTue, 26 Nov 2013 22:52:30 GMT
https://trac.sagemath.org/ticket/15456#comment:5
https://trac.sagemath.org/ticket/15456#comment:5
<p>
I take the part about it being correct in <code>CoxeterGroup</code> back. It seems to only be correct for the <code>CoxeterGroups().example()</code>.
</p>
<p>
I assume that you are right in that "Coxeter group category provides a default implementation for the other" but no one has implemented <code>has_right_descent</code> or <code>has_left_descent</code> for any of the finite or affine <code>WeylGroups</code> or <code>CoxeterGroups</code> (to an end user of these groups, this is a bug because it should just raise a <code>Not Implemented</code> error).
</p>
<p>
Is the correct solution then that
</p>
<p>
(1) someone needs to implement them for each of the types
</p>
<p>
(2) that the default implementation should be <code>has_left_descent</code> should call <code>has_descent(self, side="left")</code>
</p>
<p>
(3) the default implementation of <code>has_right_descent</code> should raise a <code>Not Implemented</code> error
</p>
<p>
(4) something else?
</p>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:6
https://trac.sagemath.org/ticket/15456#comment:6
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TicketchapotonWed, 05 Mar 2014 16:25:54 GMTstatus changed; commit, branch, author set
https://trac.sagemath.org/ticket/15456#comment:7
https://trac.sagemath.org/ticket/15456#comment:7
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>549f238c87e446116457d645c2b0171b423c02df</em>
</li>
<li><strong>branch</strong>
set to <em>u/chapoton/15456</em>
</li>
<li><strong>author</strong>
set to <em>Frédéric Chapoton</em>
</li>
</ul>
<p>
Here is a possible solution. Please review.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=549f238c87e446116457d645c2b0171b423c02df"><span class="icon"></span>549f238</a></td><td><code>trac #15456 tentative solution</code>
</td></tr></table>
TickettscrimWed, 05 Mar 2014 16:36:01 GMT
https://trac.sagemath.org/ticket/15456#comment:8
https://trac.sagemath.org/ticket/15456#comment:8
<p>
I think what we should do is have <code>has_left_descent</code> call <code>has_descent(self, side="right")</code> and <code>has_right_descent</code> call <code>has_descent(self, side="left")</code>.
</p>
TicketchapotonWed, 05 Mar 2014 16:39:48 GMT
https://trac.sagemath.org/ticket/15456#comment:9
https://trac.sagemath.org/ticket/15456#comment:9
<p>
8)
</p>
<p>
Indeed ? well, please do that if you think this is the thing to do.
</p>
TicketsdentonWed, 05 Mar 2014 16:49:19 GMT
https://trac.sagemath.org/ticket/15456#comment:10
https://trac.sagemath.org/ticket/15456#comment:10
<p>
tscrim: That's what WAS happening, but if you instantiate an abstract Weyl Group or Coxeter Group it leads to an infinite loop, which is a bug. What we need is a <a class="missing wiki">NotImplemented?</a> error if neither side is implemented, and the appropriate call to the implemented 'side' if one side or the other is concretely implemented but not the other.
</p>
<p>
I see two possible ways forward:
1) The default <a class="missing wiki">Coxeter/Weyl?</a> Group 'has_x_descent' methods just give a <a class="missing wiki">NotImplemented?</a> error, pushing it to the person creating a concrete realization to actually implement them correctly and not give an infinite loop, or
2) Introduce some kind of '_is_implemented' flags for checking left and right descent, which are false by default, and set to true if there's a concrete implementation on one side that the other can use. Then if it's true, we call the other side, and otherwise raise a <a class="missing wiki">NotImplemented?</a> error.
</p>
<p>
Option 2 seems doable but introduces an Obscure Thing for the implementers of Coxeter groups to keep track of. As such, I would advocate for Option 1.
</p>
TicketsdentonWed, 05 Mar 2014 16:52:45 GMT
https://trac.sagemath.org/ticket/15456#comment:11
https://trac.sagemath.org/ticket/15456#comment:11
<p>
It's also worth noting that sometimes one 'side' or the other is simply faster for computing descents. This is clearly true for simple permutations: one can check in constant time if there's a right descent, but it takes O(n) to check for a left descent. So then it would be bad news bears to have a default implementation of the right descent that calls the left decent.
</p>
<p>
But in other cases, it's maybe the left descent that's faster to compute, so you wouldn't want the reverse situation, either...
</p>
<p>
So I think I would advocate against breaking symmetry at the Category level, and just raise <a class="missing wiki">NotImplemented?</a> errors for both sides.
</p>
TicketchapotonWed, 05 Mar 2014 17:13:09 GMT
https://trac.sagemath.org/ticket/15456#comment:12
https://trac.sagemath.org/ticket/15456#comment:12
<p>
I do not understand why my solution is not good enough. From my point of view, the function <code>has_descent</code> in <code>weyl_group.py</code> should rather be named <code>has_right_descent</code>, because it is necessary to have at least one of the two (left or right) descent methods to avoid the infinite loop. I think that <code>has_descent</code> should only be defined for abstract Coxeter groups.
</p>
TickettscrimWed, 05 Mar 2014 17:20:28 GMT
https://trac.sagemath.org/ticket/15456#comment:13
https://trac.sagemath.org/ticket/15456#comment:13
<p>
Hey Tom,
</p>
<p>
If we made both left/right <code>@abstract_method</code>'s, then we would have to implement all methods. If we made them optional, I feel that we'd get bug reports about these not being implemented. The problem is that <code>has_descent()</code> wasn't being brought into the loop, so if that was the only thing implemented, then one couldn't use <code>has_left_descent()</code> as expects. Thus <a class="ticket" href="https://trac.sagemath.org/ticket/15456#comment:4" title="Comment 4">the spec</a>:
</p>
<blockquote class="citation">
<ul><li>at least one of has_descent / has_right_descent / has_left_descent should be implemented
</li></ul></blockquote>
<p>
would be satisfied in my proposal.
</p>
<p>
Although I think we could do a modified version of your proposal 1) by implementing some kind of modification to <code>@abstract_method</code>. Something like
</p>
<pre class="wiki">@abstract_method(circular=['foo', 'bar'])
</pre><p>
where if <code>foo</code> and <code>bar</code> were also called, then error out.
</p>
TickettscrimWed, 05 Mar 2014 17:25:48 GMT
https://trac.sagemath.org/ticket/15456#comment:14
https://trac.sagemath.org/ticket/15456#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/15456#comment:12" title="Comment 12">chapoton</a>:
</p>
<blockquote class="citation">
<p>
I do not understand why my solution is not good enough. From my point of view, the function <code>has_descent</code> in <code>weyl_group.py</code> should rather be named <code>has_right_descent</code>, because it is necessary to have at least one of the two (left or right) descent methods to avoid the infinite loop. I think that <code>has_descent</code> should only be defined for abstract Coxeter groups.
</p>
</blockquote>
<p>
The <code>has_descent()</code> in <code>weyl_group.py</code> works for both sides, and by simply doing an alias, you're exposing invalid (at least 'should not be used') arguments in <code>has_right_descent()</code>. In essence what you're doing is saying <code>has_right_descent()</code> should call <code>has_descent()</code> with the appropriate arguments. I'm saying let's do this in a generic fashion to follow the spec (I think <a class="ticket" href="https://trac.sagemath.org/ticket/15456#comment:4" title="Comment 4">comment:4</a> point 1 is in the doc already, but I haven't checked).
</p>
TicketzabrockiWed, 05 Mar 2014 17:40:24 GMT
https://trac.sagemath.org/ticket/15456#comment:15
https://trac.sagemath.org/ticket/15456#comment:15
<p>
I couldn't find an indication that the documentation specifies that at least one of has_descent / has_right_descent / has_left_descent should be implemented.
</p>
<p>
Instead it seems to be that the default implementation of <code>has_descent</code> calls <code>has_left</code>or<code>right_descent</code>
</p>
<p>
Maybe we should throw checks in <code>_test_has_descent</code> to ensure that all of the functions work.
</p>
TicketgitWed, 05 Mar 2014 20:25:40 GMTcommit changed
https://trac.sagemath.org/ticket/15456#comment:16
https://trac.sagemath.org/ticket/15456#comment:16
<ul>
<li><strong>commit</strong>
changed from <em>549f238c87e446116457d645c2b0171b423c02df</em> to <em>d0bdccadb38c68a0019d2056daebfd601b8910cb</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d0bdccadb38c68a0019d2056daebfd601b8910cb"><span class="icon"></span>d0bdcca</a></td><td><code>trac #15456 another try</code>
</td></tr></table>
TicketchapotonWed, 05 Mar 2014 20:30:16 GMT
https://trac.sagemath.org/ticket/15456#comment:17
https://trac.sagemath.org/ticket/15456#comment:17
<p>
Here is new (better) proposal:
</p>
<p>
The main idea is "please never override has_descent" and "please implement either has_left_descent or has_right_descent" in every instance (if only as a place holder raising <code>NotImplementedError</code>).
</p>
<p>
In <code>categories/coxeter_group</code>, there is only the mechanism:
</p>
<ul><li>has_descent calls either has_left_descent or has right_descent
</li><li>they call each other
</li></ul><p>
In instances of Coxeter group, one implements either has_left_descent or has_right_descent or both. Even in a minimalistic way, raising a <code>NotImplementedError</code>.
</p>
<p>
What do you think ?
</p>
TicketgitWed, 05 Mar 2014 20:34:15 GMTcommit changed
https://trac.sagemath.org/ticket/15456#comment:18
https://trac.sagemath.org/ticket/15456#comment:18
<ul>
<li><strong>commit</strong>
changed from <em>d0bdccadb38c68a0019d2056daebfd601b8910cb</em> to <em>bef966f7e6f1ce42da5578c8da4df8d5ebdf9f9b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=bef966f7e6f1ce42da5578c8da4df8d5ebdf9f9b"><span class="icon"></span>bef966f</a></td><td><code>trac #15456 remove two unused lines</code>
</td></tr></table>
TicketzabrockiThu, 06 Mar 2014 12:11:32 GMT
https://trac.sagemath.org/ticket/15456#comment:19
https://trac.sagemath.org/ticket/15456#comment:19
<p>
I think this is a good solution and probably what was intended to happen when the <code>CoxeterGroup</code> code was originally written. Do others agree?
</p>
TickettscrimThu, 06 Mar 2014 18:32:54 GMT
https://trac.sagemath.org/ticket/15456#comment:20
https://trac.sagemath.org/ticket/15456#comment:20
<p>
I don't like having the warning saying "don't override this", especially since I don't see a good reason for doing this (and there's no "final" semantic in python). I am in favor of saying we must implement either <code>has_left_descent()</code> or <code>has_right_descent()</code>. Also possibly implementing has_left/right_descent()<code> for </code>weyl_group.py<code> to call </code>has_descent()`.
</p>
<p>
Anyways, I've put my working version on trac at <code>u/tscrim/15456</code>. I also gave default implementations of <code>has_left/right_descent()</code> to <code>weyl_group.py</code> for speed. Although now that I've done the implementation, I'm less convinced of my suggestion. Perhaps what would be best is some category magic. We test upon creation of a Coxeter group to see if any of the methods have been implemented. Thus we do the following:
</p>
<ul><li>If <code>has_descent()</code> has been implemented, give default <code>has_*_descent()</code> call <code>has_descent()</code> with the appropriate args for those not implemented.
</li><li>If none were implemented, error out on construction.
</li><li>Otherwise set it up as we have it now.
</li></ul><p>
Maybe this is heavy-handed?
</p>
<p>
Nevertheless, here are some timings. I'm using the example from the ticket description. Baseline:
</p>
<pre class="wiki">sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 860 us per loop
sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 820 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 200 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 209 us per loop
</pre><p>
With Frederic's branch:
</p>
<pre class="wiki">sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 987 us per loop
sage: %timeit w.has_left_descent(3)
1000 loops, best of 3: 929 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 204 us per loop
sage: %timeit w.has_right_descent(3)
1000 loops, best of 3: 215 us per loop
</pre><p>
With my branch:
</p>
<pre class="wiki">sage: %timeit w.has_descent(3, side="left")
1000 loops, best of 3: 867 us per loop
sage: %timeit w.has_left_descent(3)
1000 loops, best of 3: 902 us per loop
sage: %timeit w.has_descent(3, side="right")
1000 loops, best of 3: 200 us per loop
sage: %timeit w.has_right_descent(3)
1000 loops, best of 3: 191 us per loop
</pre>
TicketchapotonFri, 07 Mar 2014 21:09:05 GMT
https://trac.sagemath.org/ticket/15456#comment:21
https://trac.sagemath.org/ticket/15456#comment:21
<p>
So, what shall we do here ?
</p>
<p>
If I understand your proposal, when no <code>has_something</code> is implemented in an instance of Coxeter group, one has two different possibilities of infinite loop instead of one.
</p>
<p>
Supose now that I only implement <code>has_left</code> and that I call <code>has_right</code>. This will result in an infinite loop in your proposal, no ?
</p>
TickettscrimFri, 07 Mar 2014 22:59:49 GMT
https://trac.sagemath.org/ticket/15456#comment:22
https://trac.sagemath.org/ticket/15456#comment:22
<p>
Ah, I made a correction in my implementation <code>has_right(i)</code> called <code>(~self).has_descent(i, side='left')</code>.
</p>
<p>
I'm not so sure now what the best course of action is. From the above, having to create the inverse is slow (at least generically). I'm partially inclined to just implement <code>has_left</code> and <code>has_right</code> for Weyl groups which calls <code>has_descent(i, side=*)</code> to fix the problem at present. In any of the solutions except my initial one, I would think the speed should roughly be the same...
</p>
TicketchapotonFri, 14 Mar 2014 16:30:17 GMTkeywords set
https://trac.sagemath.org/ticket/15456#comment:23
https://trac.sagemath.org/ticket/15456#comment:23
<ul>
<li><strong>keywords</strong>
<em>coxeter</em> added
</li>
</ul>
TicketgitTue, 08 Apr 2014 19:11:14 GMTcommit changed
https://trac.sagemath.org/ticket/15456#comment:24
https://trac.sagemath.org/ticket/15456#comment:24
<ul>
<li><strong>commit</strong>
changed from <em>bef966f7e6f1ce42da5578c8da4df8d5ebdf9f9b</em> to <em>c1deaa5708d0f9e08e02d24c8cb5a0a39328453b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c1deaa5708d0f9e08e02d24c8cb5a0a39328453b"><span class="icon"></span>c1deaa5</a></td><td><code>Merge branch 'u/chapoton/15456' of ssh://trac.sagemath.org:22/sage into 15456</code>
</td></tr></table>
TicketzabrockiThu, 10 Apr 2014 19:10:46 GMT
https://trac.sagemath.org/ticket/15456#comment:25
https://trac.sagemath.org/ticket/15456#comment:25
<p>
I am comfortable with accepting Frédéric's changes. From Nicolas' comments I think that the original implementation meant to have <code>has_descent</code> should call either <code>has_left</code> or <code>has_right</code> and at least one of those needs to be implemented.
</p>
<p>
I checked that at least the <code>_test_has_descent</code> will fail when one of these is not implemented by going into a Coxeter group implementation, deleting the <code>has_right_descent</code> method, and then checking that <code>G._test_has_descent()</code> raises an error. The problem is that all tests will pass in the implementation of a Coxeter group unless the <code>_test_has_descent</code> method is called (which was probably always the case and the source of the original problem). Is there a way of ensuring that a test of the left and right functions will be made for future implementations? We can just leave it up to the warning which is already in the documentation.
</p>
TicketgitFri, 18 Apr 2014 19:22:39 GMTcommit changed
https://trac.sagemath.org/ticket/15456#comment:26
https://trac.sagemath.org/ticket/15456#comment:26
<ul>
<li><strong>commit</strong>
changed from <em>c1deaa5708d0f9e08e02d24c8cb5a0a39328453b</em> to <em>5922bda3f092d44b86b3a32b34b1d0a64834713c</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5922bda3f092d44b86b3a32b34b1d0a64834713c"><span class="icon"></span>5922bda</a></td><td><code>Merge branch 'u/chapoton/15456' of ssh://trac.sagemath.org:22/sage into 15456</code>
</td></tr></table>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:27
https://trac.sagemath.org/ticket/15456#comment:27
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
Ticketvbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:28
https://trac.sagemath.org/ticket/15456#comment:28
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketchapotonSat, 14 Feb 2015 10:27:03 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:29
https://trac.sagemath.org/ticket/15456#comment:29
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.4</em> to <em>sage-6.5</em>
</li>
</ul>
TicketchapotonThu, 23 Apr 2015 18:17:57 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:30
https://trac.sagemath.org/ticket/15456#comment:30
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.5</em> to <em>sage-6.7</em>
</li>
</ul>
TicketchapotonFri, 22 May 2015 20:26:12 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:31
https://trac.sagemath.org/ticket/15456#comment:31
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.7</em> to <em>sage-6.8</em>
</li>
</ul>
TicketaschillingMon, 08 Jun 2015 16:02:39 GMTcommit, branch changed
https://trac.sagemath.org/ticket/15456#comment:32
https://trac.sagemath.org/ticket/15456#comment:32
<ul>
<li><strong>commit</strong>
changed from <em>5922bda3f092d44b86b3a32b34b1d0a64834713c</em> to <em>45ff0e370aea78c35197688d8983e9bfd1c00b51</em>
</li>
<li><strong>branch</strong>
changed from <em>u/chapoton/15456</em> to <em>public/15456</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=45ff0e370aea78c35197688d8983e9bfd1c00b51"><span class="icon"></span>45ff0e3</a></td><td><code>Merge branch 'develop' into u/chapoton/15456</code>
</td></tr></table>
TicketaschillingMon, 08 Jun 2015 16:55:23 GMT
https://trac.sagemath.org/ticket/15456#comment:33
https://trac.sagemath.org/ticket/15456#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/15456#comment:25" title="Comment 25">zabrocki</a>:
</p>
<blockquote class="citation">
<p>
I am comfortable with accepting Frédéric's changes. From Nicolas' comments I think that the original implementation meant to have <code>has_descent</code> should call either <code>has_left</code> or <code>has_right</code> and at least one of those needs to be implemented.
</p>
<p>
I checked that at least the <code>_test_has_descent</code> will fail when one of these is not implemented by going into a Coxeter group implementation, deleting the <code>has_right_descent</code> method, and then checking that <code>G._test_has_descent()</code> raises an error. The problem is that all tests will pass in the implementation of a Coxeter group unless the <code>_test_has_descent</code> method is called (which was probably always the case and the source of the original problem). Is there a way of ensuring that a test of the left and right functions will be made for future implementations? We can just leave it up to the warning which is already in the documentation.
</p>
</blockquote>
<p>
I am ok with Federic's solution. All tests pass, so I think this should go in.
</p>
TicketzabrockiMon, 08 Jun 2015 16:56:01 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/15456#comment:34
https://trac.sagemath.org/ticket/15456#comment:34
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Anne Schilling, Mike Zabrocki</em>
</li>
</ul>
<p>
Anne looked at the code as well. We both approve and I am setting to positive review.
</p>
TicketchapotonMon, 08 Jun 2015 17:28:43 GMT
https://trac.sagemath.org/ticket/15456#comment:35
https://trac.sagemath.org/ticket/15456#comment:35
<p>
This will conflict will <a class="closed ticket" href="https://trac.sagemath.org/ticket/18610" title="defect: Bug: Circular Descent Check in WeylGroups (closed: fixed)">#18610</a> and is probably a duplicate. The review is coming too late.
</p>
<p>
This need to be checked, nevertheless.
</p>
TicketaschillingMon, 08 Jun 2015 19:46:44 GMTstatus changed
https://trac.sagemath.org/ticket/15456#comment:36
https://trac.sagemath.org/ticket/15456#comment:36
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_info</em>
</li>
</ul>
TicketaschillingMon, 08 Jun 2015 19:47:35 GMTmilestone changed
https://trac.sagemath.org/ticket/15456#comment:37
https://trac.sagemath.org/ticket/15456#comment:37
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.8</em> to <em>sage-duplicate/invalid/wontfix</em>
</li>
</ul>
TicketzabrockiTue, 09 Jun 2015 11:58:14 GMTkeywords changed
https://trac.sagemath.org/ticket/15456#comment:38
https://trac.sagemath.org/ticket/15456#comment:38
<ul>
<li><strong>keywords</strong>
<em>sd65</em> added
</li>
</ul>
TicketchapotonWed, 24 Jun 2015 19:30:46 GMTstatus changed
https://trac.sagemath.org/ticket/15456#comment:39
https://trac.sagemath.org/ticket/15456#comment:39
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>positive_review</em>
</li>
</ul>
<p>
ok, let us consider that this has been solved by <a class="closed ticket" href="https://trac.sagemath.org/ticket/18610" title="defect: Bug: Circular Descent Check in WeylGroups (closed: fixed)">#18610</a>
</p>
TicketaschillingWed, 24 Jun 2015 19:35:36 GMT
https://trac.sagemath.org/ticket/15456#comment:40
https://trac.sagemath.org/ticket/15456#comment:40
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/15456#comment:39" title="Comment 39">chapoton</a>:
</p>
<blockquote class="citation">
<p>
ok, let us consider that this has been solved by <a class="closed ticket" href="https://trac.sagemath.org/ticket/18610" title="defect: Bug: Circular Descent Check in WeylGroups (closed: fixed)">#18610</a>
</p>
</blockquote>
<p>
Why are you setting it to positive review then and not leave it at sage-duplicate?
</p>
TicketchapotonWed, 24 Jun 2015 19:44:21 GMT
https://trac.sagemath.org/ticket/15456#comment:41
https://trac.sagemath.org/ticket/15456#comment:41
<p>
This is 'positive review as a duplicate', the usual standard procedure, so that it can be closed!
</p>
TicketvbraunFri, 17 Jul 2015 20:07:16 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/15456#comment:42
https://trac.sagemath.org/ticket/15456#comment:42
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>duplicate</em>
</li>
</ul>
Ticket