Sage: Ticket #20402: Make subword complexes compatible with real reflection groups
https://trac.sagemath.org/ticket/20402
<p>
The subword complex code was written and optimized for real reflection groups, but then made its way into Sage for Coxeter groups. This ticket is to make it handle both.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/20402
Trac 1.1.6stumpc5Sat, 09 Apr 2016 19:16:11 GMTdependencies set
https://trac.sagemath.org/ticket/20402#comment:1
https://trac.sagemath.org/ticket/20402#comment:1
<ul>
<li><strong>dependencies</strong>
set to <em>#11187</em>
</li>
</ul>
Ticketstumpc5Sat, 09 Apr 2016 20:57:31 GMTbranch set
https://trac.sagemath.org/ticket/20402#comment:2
https://trac.sagemath.org/ticket/20402#comment:2
<ul>
<li><strong>branch</strong>
set to <em>u/stumpc5/20402</em>
</li>
</ul>
TicketgitSun, 10 Apr 2016 14:03:50 GMTcommit set
https://trac.sagemath.org/ticket/20402#comment:3
https://trac.sagemath.org/ticket/20402#comment:3
<ul>
<li><strong>commit</strong>
set to <em>5ae3b7f793ad96f9f39999b158398434009435da</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c"><span class="icon"></span>2d70fab</a></td><td><code>trac #11187 more doc tuning</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a04b9c81eec89e10015c090128127ac9c4810eba"><span class="icon"></span>a04b9c8</a></td><td><code>Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9c96d33361f4cf9fbd99962ce951dfd82ff1a8fa"><span class="icon"></span>9c96d33</a></td><td><code>added ST type casting for real groups + documentation of crg's</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae"><span class="icon"></span>5b1f2b0</a></td><td><code>Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d4e7a617669d890c3332d4f76b2a61b7574f67c9"><span class="icon"></span>d4e7a61</a></td><td><code>working through the doctests for complex groups</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d7b8c7d6dfe2a7872d0ebb89da15cb89a2effda9"><span class="icon"></span>d7b8c7d</a></td><td><code>fixing all doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7c9d700e9bb25a5b88f2e509be84851b8c99431d"><span class="icon"></span>7c9d700</a></td><td><code>Merge branch 'u/stumpc5/11187' into u/stumpc5/20402</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=85d0ce105a549ef8cad40df6bf8f0d8fed3122cf"><span class="icon"></span>85d0ce1</a></td><td><code>fixed a typo in the doc</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=521d50b4da416ff471568827ea812c471354558c"><span class="icon"></span>521d50b</a></td><td><code>Merge branch 'u/stumpc5/11187' into u/stumpc5/20402</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5ae3b7f793ad96f9f39999b158398434009435da"><span class="icon"></span>5ae3b7f</a></td><td><code>marked a missing doctest + fixed a bug to show type C2</code>
</td></tr></table>
TicketgitSun, 10 Apr 2016 18:47:20 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:4
https://trac.sagemath.org/ticket/20402#comment:4
<ul>
<li><strong>commit</strong>
changed from <em>5ae3b7f793ad96f9f39999b158398434009435da</em> to <em>df55cdd653d443594a0726b4e1e6e97c539fb0e9</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=72082b119626d3b1477cdda10c1a4ac24192716b"><span class="icon"></span>72082b1</a></td><td><code>fixed a bug in noncrossing partition lattice and reflection/absolute order</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=844f4506ca67cb2ae69e04ca71960cb08f6b0d33"><span class="icon"></span>844f450</a></td><td><code>Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d4256691756e16c64b6f2f855367f5afb911a00c"><span class="icon"></span>d425669</a></td><td><code>Fixing failures and some other things.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=87cd989b6d1dd7e5e8704e5c68d4ba2240474234"><span class="icon"></span>87cd989</a></td><td><code>fixed one doctest in category complex reflection group</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=05a8ab4201355f554d4c7e0916b1f90c1ab1ff6d"><span class="icon"></span>05a8ab4</a></td><td><code>11187: fixed trivial doctest failures</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=32bf1aeb9a730335d0f17744349ef26838a3397a"><span class="icon"></span>32bf1ae</a></td><td><code>11187: cleanup of the organization of the various axioms (WellGenerated, ...) for complex/generalized reflection groups + documentation improvements</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=73276caaa350dcfb7c00999f0a4b84626f19c75a"><span class="icon"></span>73276ca</a></td><td><code>Merge branch 'u/tscrim/11187' of trac.sagemath.org:sage into t/11187/11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e074050131cd483022c5783f749cbce3ac522ae5"><span class="icon"></span>e074050</a></td><td><code>merged</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b572c74c3a4db2909bea5c538c8be26089068fc6"><span class="icon"></span>b572c74</a></td><td><code>fixed a missing doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=df55cdd653d443594a0726b4e1e6e97c539fb0e9"><span class="icon"></span>df55cdd</a></td><td><code>merged the newest version of 11187</code>
</td></tr></table>
TicketgitSun, 10 Apr 2016 19:05:39 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:5
https://trac.sagemath.org/ticket/20402#comment:5
<ul>
<li><strong>commit</strong>
changed from <em>df55cdd653d443594a0726b4e1e6e97c539fb0e9</em> to <em>19271510dd0f4b3ea6480fb9e6d15d70383440fc</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=19271510dd0f4b3ea6480fb9e6d15d70383440fc"><span class="icon"></span>1927151</a></td><td><code>fixed a bug in simple_root_index</code>
</td></tr></table>
Ticketstumpc5Sun, 10 Apr 2016 19:07:59 GMT
https://trac.sagemath.org/ticket/20402#comment:6
https://trac.sagemath.org/ticket/20402#comment:6
<p>
This ticket seems to be ready once <a class="closed ticket" href="https://trac.sagemath.org/ticket/11187" title="enhancement: Implementation of finite reflection groups (closed: fixed)">#11187</a> has positive review.
</p>
TicketchapotonMon, 11 Apr 2016 09:21:21 GMT
https://trac.sagemath.org/ticket/20402#comment:7
https://trac.sagemath.org/ticket/20402#comment:7
<p>
This is failing:
</p>
<pre class="wiki">sage: a3r=ReflectionGroup(['A',3], base_ring=QQ)
sage: sr=SubwordComplex([1,3,2,1,3,2,1,3,2],a3r.w0)
sage: fr=sr.greedy_facet()
sage: fr.extended_weight_configuration()
</pre>
TicketgitWed, 13 Apr 2016 11:30:19 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:8
https://trac.sagemath.org/ticket/20402#comment:8
<ul>
<li><strong>commit</strong>
changed from <em>19271510dd0f4b3ea6480fb9e6d15d70383440fc</em> to <em>d6bc459825560776ea95c8d3e34d32b2fb78d059</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0becba3698653fedcfa8547d44fc7d45fb2fecb3"><span class="icon"></span>0becba3</a></td><td><code>Merge branch 'public/11187' of git://trac.sagemath.org/sage into u/stumpc5/20396</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8927ab98d9aa88610774567e4bde0ce8ec8c14b9"><span class="icon"></span>8927ab9</a></td><td><code>matrix action is on the right!</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=eb2ea75a566bccb38fce37ba72f593539cc1c1a5"><span class="icon"></span>eb2ea75</a></td><td><code>cleaned the reduced word code</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=6fbba185faebad9f9d39028cd6002fee9315eb00"><span class="icon"></span>6fbba18</a></td><td><code>undo the change to repr methods</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8497f5d6aebbd8dd640ea41ffbd904c47886e34e"><span class="icon"></span>8497f5d</a></td><td><code>first try to skip elements in the iteration, too slow</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=def701588736edda81a86ddd1a9cbdbc849657be"><span class="icon"></span>def7015</a></td><td><code>reorganized the invariant form (still missing an improvement) + fixed two doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ab73bc35b90d0b51f1304a9c3368468a2323fe34"><span class="icon"></span>ab73bc3</a></td><td><code>Merge branch 'u/stumpc5/11187' into u/stumpc5/20402</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b5a2c8d029450ce3c94b7c14337aaa62c6e0b693"><span class="icon"></span>b5a2c8d</a></td><td><code>moved all examles back to ReflectionGroup</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e36ddad9e601f87d42c0488c2a75f13a0fee6ac9"><span class="icon"></span>e36ddad</a></td><td><code>added fundamental_weight to coxetergroup implementation</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d6bc459825560776ea95c8d3e34d32b2fb78d059"><span class="icon"></span>d6bc459</a></td><td><code>made all doctests work, but I used a dirty hack to solve the weight configuration</code>
</td></tr></table>
TicketgitWed, 20 Apr 2016 07:19:53 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:9
https://trac.sagemath.org/ticket/20402#comment:9
<ul>
<li><strong>commit</strong>
changed from <em>d6bc459825560776ea95c8d3e34d32b2fb78d059</em> to <em>a1b3d3ba5055e4217d8cecd5b09bd51b078802f2</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=dc0106628d58cb263866b9cd601696ea71f18e9f"><span class="icon"></span>dc01066</a></td><td><code>started to add 'optional - gap3'</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=fd0c9cc616a03de20a12aba530c96e73f8f60911"><span class="icon"></span>fd0c9cc</a></td><td><code>added optional gap3</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1537a795f6d7a86f8c3a7a79e86d4b31405de323"><span class="icon"></span>1537a79</a></td><td><code>added optional gap3</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a60cf31d47777353bcae92665d5e5e739fc02a29"><span class="icon"></span>a60cf31</a></td><td><code>added some missing optional gap3</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=feebf975713fcb5051f55d47cbe090de45fe5a7d"><span class="icon"></span>feebf97</a></td><td><code>finalized the invariant form</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4955d68d9e4dbde2d1425afbae36e619c2d6ba14"><span class="icon"></span>4955d68</a></td><td><code>added the missing optional gap3 in the cython file</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c5c43bcba449039892cecb0d18238b4bf87e8a7f"><span class="icon"></span>c5c43bc</a></td><td><code>the next round of optional gap3 insertions</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=244e6ff6b516b768c01031817dacc75c05ea889a"><span class="icon"></span>244e6ff</a></td><td><code>Merge branch 'public/11187' of trac.sagemath.org:sage into public/reflection_groups-11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f541349eae7b8b3b6a61c57aaafd6bb8c84b3265"><span class="icon"></span>f541349</a></td><td><code>Some last little beautification.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a1b3d3ba5055e4217d8cecd5b09bd51b078802f2"><span class="icon"></span>a1b3d3b</a></td><td><code>Merge branch 'u/stumpc5/11187' into u/stumpc5/20402</code>
</td></tr></table>
TicketgitSun, 24 Apr 2016 06:18:16 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:10
https://trac.sagemath.org/ticket/20402#comment:10
<ul>
<li><strong>commit</strong>
changed from <em>a1b3d3ba5055e4217d8cecd5b09bd51b078802f2</em> to <em>e63d2ece579d1afd1df9c4afcf951407e6fbe36d</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=0db28d6c6e64818eb32fb9826ff7a41333c814d6"><span class="icon"></span>0db28d6</a></td><td><code>fixed doctests that have not been run last week!</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7afc02c78f5c0aa58d5b638e20c135678a3bec7e"><span class="icon"></span>7afc02c</a></td><td><code>Fixing doctests in combinat/root_system/coxeter_group.py.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=02694256c73520e7f4dd80c455cdae133be858a7"><span class="icon"></span>0269425</a></td><td><code>Blanket fix bare except block.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9b0ef927c17789f2dbeea432b8316f99d138cb88"><span class="icon"></span>9b0ef92</a></td><td><code>Making it an AttributeError.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=87cdde051dabb9eb963dd3614aecdcd851363bb3"><span class="icon"></span>87cdde0</a></td><td><code>Merge branch 'public/11187' into u/stumpc5/20402</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8d765204ab597ed2fd1570c2438d5fda3eb60192"><span class="icon"></span>8d76520</a></td><td><code>using a cached version self._number_of_reflections to avoid computing it again and again, expecially important in has_descent</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0b6d895c865ebdb319de6d58e862e7ae1a1cb784"><span class="icon"></span>0b6d895</a></td><td><code>Merge branch 'develop' into public/11187</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e63d2ece579d1afd1df9c4afcf951407e6fbe36d"><span class="icon"></span>e63d2ec</a></td><td><code>Merge branch 'public/11187' into u/stumpc5/20402</code>
</td></tr></table>
Ticketstumpc5Mon, 25 Apr 2016 10:03:39 GMTsummary changed
https://trac.sagemath.org/ticket/20402#comment:11
https://trac.sagemath.org/ticket/20402#comment:11
<ul>
<li><strong>summary</strong>
changed from <em>Make real reflection group compatible with subword complex</em> to <em>Make subword complexes compatible with real reflection groups</em>
</li>
</ul>
TicketgitMon, 25 Apr 2016 10:09:36 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:12
https://trac.sagemath.org/ticket/20402#comment:12
<ul>
<li><strong>commit</strong>
changed from <em>e63d2ece579d1afd1df9c4afcf951407e6fbe36d</em> to <em>0b81cd6ccf77c652672ca75aa693123ca77bd93c</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=4bacaea70cc01123f2c4b0a2bad070d971721bf4"><span class="icon"></span>4bacaea</a></td><td><code>trivial typo</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=fc8a0aa14096fa1006e6aeea5da58304cd4c85d8"><span class="icon"></span>fc8a0aa</a></td><td><code>replaced the wrongly assumed right action by a left action</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ef6dbace9fbe0e4c4dcca656cb514cf0f89b3ebd"><span class="icon"></span>ef6dbac</a></td><td><code>fixed the action on root indices with left and right version</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=308f26b7aeb9da40662d4821ecc5f82a498ec204"><span class="icon"></span>308f26b</a></td><td><code>added optional gap3 everywhere</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0b81cd6ccf77c652672ca75aa693123ca77bd93c"><span class="icon"></span>0b81cd6</a></td><td><code>added a few missing optional gap3</code>
</td></tr></table>
Ticketstumpc5Mon, 25 Apr 2016 10:10:18 GMTstatus changed
https://trac.sagemath.org/ticket/20402#comment:13
https://trac.sagemath.org/ticket/20402#comment:13
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
Ticketstumpc5Mon, 25 Apr 2016 10:10:46 GMT
https://trac.sagemath.org/ticket/20402#comment:14
https://trac.sagemath.org/ticket/20402#comment:14
<p>
I think this is ready to go once the patchbots are happy!
</p>
Ticketstumpc5Mon, 25 Apr 2016 10:13:11 GMTkeywords changed
https://trac.sagemath.org/ticket/20402#comment:15
https://trac.sagemath.org/ticket/20402#comment:15
<ul>
<li><strong>keywords</strong>
<em>reflection</em> <em>days80</em> added
</li>
</ul>
TicketgitMon, 25 Apr 2016 12:44:10 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:16
https://trac.sagemath.org/ticket/20402#comment:16
<ul>
<li><strong>commit</strong>
changed from <em>0b81cd6ccf77c652672ca75aa693123ca77bd93c</em> to <em>0ee1f3bf9c772352b9a7994afc7f4359e618b309</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=0ee1f3bf9c772352b9a7994afc7f4359e618b309"><span class="icon"></span>0ee1f3b</a></td><td><code>added a few more missing optional gap3</code>
</td></tr></table>
TickettscrimMon, 25 Apr 2016 14:02:23 GMTstatus changed
https://trac.sagemath.org/ticket/20402#comment:17
https://trac.sagemath.org/ticket/20402#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
-1 to changing all of the examples to <em>only</em> using GAP3/reflection groups. Until GAP3 becomes an optional spkg, it means they become completely untested. Moreover, it is good to make sure the functionality works between different implementations.
</p>
<p>
I think there are a few things in here which need to be abstracted up to the category, root system code, and/or Coxeter type code. For example, <code>is_crystallographic</code> should be called on the Coxeter type, of which <code>ReflectionGroup</code> does not have and should (or perhaps the Coxeter matrix).
</p>
<p>
I also do not see the reason why this change is needed:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- Lambda = {li: coeff[li] * Lambda[li] for li in Lambda}
</span><span class="gi">+ Lambda = {li: coeff[li] * Lambda[li] for li in Lambda.keys()}
</span></pre></div></div><p>
as the iteration for a dictionary is over its keys (and the former is faster and more memory efficient). With this change:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- V_weights.append(pi * fund_weight)
</span><span class="gi">+ # TODO: little hack to distinguish the implementations
+ # for ReflectionGroups and CoxeterGroup
+ from sage.groups.perm_gps.permgroup_element import PermutationGroupElemen
+ if isinstance(pi,PermutationGroupElement):
+ V_weights.append( sum(fund_weight[j]*Phi[ (~pi)(j+1)-1 ] for j in ran
+ else:
+ V_weights.append(pi * fund_weight)
</span></pre></div></div><p>
I think it would be better to have the fundamental weights handle the action of the reflection group element.
</p>
Ticketstumpc5Mon, 25 Apr 2016 14:29:31 GMT
https://trac.sagemath.org/ticket/20402#comment:18
https://trac.sagemath.org/ticket/20402#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:17" title="Comment 17">tscrim</a>:
</p>
<blockquote class="citation">
<p>
-1 to changing all of the examples to <em>only</em> using GAP3/reflection groups. Moreover, it is good to make sure the functionality works between different implementations.
</p>
</blockquote>
<p>
The header contains tests that the core functionality works for Coxeter groups, maybe one could add a more elaborate example there, providing examples for more functionality.
</p>
<p>
But I clearly wrote the code the way I did in close relationshiop to the implementation of reflection groups, and I must confess that I prefer the speed (and indeed I need that speed for my research) over the code being usable by other implementations. I think, it is okay to have the code sort of working for <code>CoxeterGroup</code>, but for example the necessity to introduce a method <code>action_on_root_indices</code> for those tells that there is actually more work to be done to make the implementation work with both without giving up the speed on one, and not introducing unnecessary function overhead on the other.
</p>
<blockquote class="citation">
<p>
Until GAP3 becomes an optional spkg, it means they become completely untested.
</p>
</blockquote>
<p>
This should be done soon in my eyes, but I understand that so far I am the only one being desperate to have it.
</p>
<blockquote class="citation">
<p>
I think there are a few things in here which need to be abstracted up to the category, root system code, and/or Coxeter type code. For example, <code>is_crystallographic</code> should be called on the Coxeter type, of which <code>ReflectionGroup</code> does not have and should (or perhaps the Coxeter matrix).
</p>
</blockquote>
<p>
A reflection group acting on a real vector space can have the property of being crystallographic or not (whether or not it stabilizes a lattice). I thus think the group itself should have this property.
</p>
<p>
What do you think of (me) adding this method also to Coxeter groups (in this ticket or another)?
</p>
<blockquote class="citation">
<p>
I also do not see the reason why this change is needed:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- Lambda = {li: coeff[li] * Lambda[li] for li in Lambda}
</span><span class="gi">+ Lambda = {li: coeff[li] * Lambda[li] for li in Lambda.keys()}
</span></pre></div></div><p>
as the iteration for a dictionary is over its keys (and the former is faster and more memory efficient).
</p>
</blockquote>
<p>
As we have per default that simple reflections are families rather than dicts (so that iterating over them actually iterates over them), the reflection group implementation also have simple roots and fundamental weights as finite families, so iterating goes over the values. That's what the <code>keys</code> is there for.
</p>
<blockquote class="citation">
<p>
With this change:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- V_weights.append(pi * fund_weight)
</span><span class="gi">+ # TODO: little hack to distinguish the implementations
+ # for ReflectionGroups and CoxeterGroup
</span></pre></div></div><p>
I think it would be better to have the fundamental weights handle the action of the reflection group element.
</p>
</blockquote>
<p>
If you mean, one might have a method <code>act on weight</code>, one could think about it, but the point here is that the action on fundamental weights can be quickly understood using the action on roots, while the general action on weights is not as simple.
</p>
<p>
I can implement a method <code>act_on_fundamental_weight</code> to factor out the hack, but is that what we actually want?
</p>
TicketgitMon, 25 Apr 2016 14:40:19 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:19
https://trac.sagemath.org/ticket/20402#comment:19
<ul>
<li><strong>commit</strong>
changed from <em>0ee1f3bf9c772352b9a7994afc7f4359e618b309</em> to <em>2c95a41c537c6625ed6abdf03fd5bbedc9da21cb</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=2c95a41c537c6625ed6abdf03fd5bbedc9da21cb"><span class="icon"></span>2c95a41</a></td><td><code>added a few more test cases for CoxeterGroup to the header</code>
</td></tr></table>
Ticketstumpc5Mon, 25 Apr 2016 14:46:06 GMT
https://trac.sagemath.org/ticket/20402#comment:20
https://trac.sagemath.org/ticket/20402#comment:20
<blockquote class="citation">
<p>
A reflection group acting on a real vector space can have the property of being crystallographic or not (whether or not it stabilizes a lattice). I thus think the group itself should have this property.
</p>
<p>
What do you think of (me) adding this method also to Coxeter groups (in this ticket or another)?
</p>
</blockquote>
<p>
I gonna for now also use
</p>
<pre class="wiki">sage: W = ReflectionGroup(['A',3])
sage: W.coxeter_matrix().is_crystallographic()
True
sage: W = CoxeterGroup(['A',3])
sage: W.coxeter_matrix().is_crystallographic()
True
</pre><p>
(without giving up the idea that the property is attached to the group itself).
</p>
TicketgitMon, 25 Apr 2016 14:52:42 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:21
https://trac.sagemath.org/ticket/20402#comment:21
<ul>
<li><strong>commit</strong>
changed from <em>2c95a41c537c6625ed6abdf03fd5bbedc9da21cb</em> to <em>803d8b9226e376cbd777f4bb1d1ef67342d289fd</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=803d8b9226e376cbd777f4bb1d1ef67342d289fd"><span class="icon"></span>803d8b9</a></td><td><code>replace a hack by using G.coxeter_matrix().is_crystallogrphic()</code>
</td></tr></table>
Ticketstumpc5Mon, 25 Apr 2016 14:59:30 GMT
https://trac.sagemath.org/ticket/20402#comment:22
https://trac.sagemath.org/ticket/20402#comment:22
<blockquote class="citation">
<p>
If you mean, one might have a method <code>act on weight</code>, one could think about it, but the point here is that the action on fundamental weights can be quickly understood using the action on roots, while the general action on weights is not as simple.
</p>
</blockquote>
<p>
Let me take that back for now, I'll get back to it...
</p>
TicketgitMon, 25 Apr 2016 18:58:45 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:23
https://trac.sagemath.org/ticket/20402#comment:23
<ul>
<li><strong>commit</strong>
changed from <em>803d8b9226e376cbd777f4bb1d1ef67342d289fd</em> to <em>52c39d9ef149ee6826cb3f8f06b7aa7582d49f14</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=bc23f814716a1f0aa947e5580289acac611534fc"><span class="icon"></span>bc23f81</a></td><td><code>fixed the action on root indices, roots, and general vectors</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0ecefc2c7cd7b1ab2a611c17b13ef06371659269"><span class="icon"></span>0ecefc2</a></td><td><code>renamed action_on_root_indices to act_on_root_indices for name unifications</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=63639a6e3041d14e2bd872858bf58abf8b03815c"><span class="icon"></span>63639a6</a></td><td><code>removed a fixed todo</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f12a61ef4cc0567e40052ba267bc1e24ce2f5dea"><span class="icon"></span>f12a61e</a></td><td><code>added method 'act' to CoxeterGroup and used it in the action on weights, also marked a few more doctests as optional gap3</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=52c39d9ef149ee6826cb3f8f06b7aa7582d49f14"><span class="icon"></span>52c39d9</a></td><td><code>fixed one broken doctest in coxeter_group.py</code>
</td></tr></table>
Ticketstumpc5Mon, 25 Apr 2016 19:03:32 GMTstatus changed
https://trac.sagemath.org/ticket/20402#comment:24
https://trac.sagemath.org/ticket/20402#comment:24
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Hi Travis, I believe I fixed everything you mentioned except for the fact that I almost completely test using <code>ReflectionGroup</code>'s. In particular, I fixed a few hacks and <code>todo</code>'s.
</p>
<p>
The changes to <code>reflection_group_real.py</code> and to <code>coxeter_group.py</code> are not strictly there to get subword complexes to work (though the uniformization is done for that reason). Should I extect those changes into a different ticket, or is everyone okay with having it here?
</p>
<p>
Beside that, I set it back to <code>needs review</code>.
</p>
TickettscrimTue, 26 Apr 2016 04:17:07 GMTstatus changed
https://trac.sagemath.org/ticket/20402#comment:25
https://trac.sagemath.org/ticket/20402#comment:25
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
It's looking a lot better Christian. Thank you.
</p>
<p>
However, let me be a bit more stern, I will not set this ticket to a positive review when you essentially remove doctest coverage because <code>gap3</code> is not an optional package yet (although this argument weakens once it is). Moreover, you can test all of the functionality with standard Sage, which such tests should be localized to each method, not (only) in a class-level test which does the "core" functionality.
</p>
<p>
For the speed issues, how much of this is making a difference? If it does, you can separate out the important functionality into a separate method, and then subclass the general class which special cases the methods when you use <code>ReflectionGroup</code>.
</p>
<p>
You are right about the issue with <code>Lambda</code>.
</p>
<p>
By renaming <code>action_on_root_indices</code>, you need to deprecate it.
</p>
<p>
I am okay with you adding an <code>is_crystallographic</code> to the group here, but it might be a little strange if the crystallographic-ness of a Coxeter group is different than its Coxeter type/matrix.
</p>
<p>
Also, instead of <code>act</code>, IIRC there is a method, which I believe is <code>_act_on_</code> (not to be confused with <code>_acted_upon_</code>), that automatically gets checked by the coercion framework. So it gives you <code>w * la</code> essentially for free. Although
</p>
TicketgitTue, 26 Apr 2016 08:01:39 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:26
https://trac.sagemath.org/ticket/20402#comment:26
<ul>
<li><strong>commit</strong>
changed from <em>52c39d9ef149ee6826cb3f8f06b7aa7582d49f14</em> to <em>283ccf598079150e16406adc6a5a55b519f8ec26</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=36ea30e9b2897f6dba40b832dadaca4a02e4af01"><span class="icon"></span>36ea30e</a></td><td><code>added _act_on_ to reflection_group_real, fixed a confusion about left/right action, fixed a typo</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=283ccf598079150e16406adc6a5a55b519f8ec26"><span class="icon"></span>283ccf5</a></td><td><code>using _act_on_ in the subword complex code, and remove the act method for CoxeterGroup again</code>
</td></tr></table>
Ticketstumpc5Tue, 26 Apr 2016 08:33:35 GMT
https://trac.sagemath.org/ticket/20402#comment:27
https://trac.sagemath.org/ticket/20402#comment:27
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:25" title="Comment 25">tscrim</a>:
</p>
<blockquote class="citation">
<p>
For the speed issues, how much of this is making a difference?
</p>
</blockquote>
<pre class="wiki">sage: W = ReflectionGroup(['A',5])
sage: c = W.index_set(); Q = c + tuple(W.w0.coxeter_sorting_word(c))
sage: %time S = SubwordComplex(Q,W.w0)
CPU times: user 42.7 ms, sys: 612 µs, total: 43.3 ms
Wall time: 39.5 ms
sage: len(S)
132
sage: W = CoxeterGroup(['A',5])
sage: c = W.index_set(); Q = c + tuple(W.w0.coxeter_sorting_word(c))
sage: %time S = SubwordComplex(Q,W.w0)
CPU times: user 15.1 s, sys: 84.3 ms, total: 15.1 s
Wall time: 15.1 s
sage: len(S)
132
</pre><p>
The complete implementation depends on <code>has_descents</code> and multiplication of elements to be fast (and this will get another boost for <code>ReflectionGroup</code> in <a class="closed ticket" href="https://trac.sagemath.org/ticket/20484" title="enhancement: Use the cythonized versions of has_descent, first_descent, descents ... (closed: invalid)">#20484</a>), when you <code>%prun</code> the code for <code>CoxeterGroup</code> you don't see a single bottleneck but that it is too slow in many parts (I am not complaining about this speed issue in general, this implementation is uniform for all Coxeter systems, finite or infinite, so that's great and the speed is what one gets there!)
</p>
<blockquote class="citation">
<p>
If it does, you can separate out the important functionality into a separate method, and then subclass the general class which special cases the methods when you use <code>ReflectionGroup</code>.
</p>
</blockquote>
<p>
This implementation of subword complexes is <strong>not</strong> a general implementation that could be moved up the hierarchy, and then a few methods could be replaced for concrete implementations. All the internal core methods are written so that internally it only plays with numbers and permutations of numbers, you only see roots popping out when using the API.
</p>
<p>
Is is indeed possible to write a toy (in terms of speed) implementation for the category of <code>CoxeterGroups</code>, but I am not the one doing that at the moment -- what I provide is a research level implementation for <code>ReflectionGroup</code>.
</p>
<blockquote class="citation">
<p>
However, let me be a bit more stern, I will not set this ticket to a positive review when you essentially remove doctest coverage because <code>gap3</code> is not an optional package yet (although this argument weakens once it is). Moreover, you can test all of the functionality with standard Sage, which such tests should be localized to each method, not (only) in a class-level test which does the "core" functionality.
</p>
</blockquote>
<p>
Hm -- you see my complaints about that above. On the other hand, I also see your point of getting rotten untested code after a while, and I don't really care if we pollute the documentation with more tests.
</p>
<blockquote class="citation">
<p>
By renaming <code>action_on_root_indices</code>, you need to deprecate it.
</p>
</blockquote>
<p>
It was only there for a few months, see <a class="closed ticket" href="https://trac.sagemath.org/ticket/11010" title="enhancement: Implementation of the SubwordComplex as defined by Knutson and Miller (closed: fixed)">#11010</a>, and this method is almost surely not used by anyone so far. You still think it should be deprecated ?
</p>
<blockquote class="citation">
<p>
I am okay with you adding an <code>is_crystallographic</code> to the group here, but it might be a little strange if the crystallographic-ness of a Coxeter group is different than its Coxeter type/matrix.
</p>
</blockquote>
<p>
How could crystallographic-ness being different for the group and for its Coxeter type ? See Section 2.8 in Humphreys Coxeter group book for a "proof" that the two are the same. Or am I overseeing something?
</p>
Ticketstumpc5Tue, 26 Apr 2016 08:34:00 GMT
https://trac.sagemath.org/ticket/20402#comment:28
https://trac.sagemath.org/ticket/20402#comment:28
<blockquote class="citation">
<p>
Also, instead of <code>act</code>, IIRC there is a method, which I believe is <code>_act_on_</code> (not to be confused with <code>_acted_upon_</code>), that automatically gets checked by the coercion framework. So it gives you <code>w * la</code> essentially for free.
</p>
</blockquote>
<p>
Done, I must admit that I like that way of having an element w in W act on vectors, while I am nervous about this implicitness in the construction that will likely result in bugs.
</p>
<blockquote class="citation">
<p>
Although
</p>
</blockquote>
<p>
missing sentence?
</p>
TicketnthieryTue, 26 Apr 2016 09:31:01 GMT
https://trac.sagemath.org/ticket/20402#comment:29
https://trac.sagemath.org/ticket/20402#comment:29
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:28" title="Comment 28">stumpc5</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Also, instead of <code>act</code>, IIRC there is a method, which I believe is <code>_act_on_</code> (not to be confused with <code>_acted_upon_</code>), that automatically gets checked by the coercion framework. So it gives you <code>w * la</code> essentially for free.
</p>
</blockquote>
<p>
Done, I must admit that I like that way of having an element w in W act on vectors, while I am nervous about this implicitness in the construction that will likely result in bugs.
</p>
</blockquote>
<p>
Same here; it's nice to have a short hand for actions (and to benefit from coercion!), but I am also uncomfortable giving yet another meaning to the operator '*'.
</p>
<p>
In any cases, we should keep consistency with Weyl groups. It's currently <code>w.action(l)</code> there (not <code>w.act(l)</code> btw); if we switch to <code>_act_on_</code>, then we should do it consistently everywhere.
</p>
<p>
Cheers,
</p>
TicketnthieryTue, 26 Apr 2016 09:32:47 GMT
https://trac.sagemath.org/ticket/20402#comment:30
https://trac.sagemath.org/ticket/20402#comment:30
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:27" title="Comment 27">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
How could crystallographic-ness being different for the group and for its Coxeter type ? See Section 2.8 in Humphreys Coxeter group book for a "proof" that the two are the same.
</p>
</blockquote>
<p>
I guess that's Travis's point: mathematically, they two should be the same. If there are two methods implementing this fact, then we need to make sure that they remain consistent.
</p>
Ticketstumpc5Tue, 26 Apr 2016 10:26:28 GMT
https://trac.sagemath.org/ticket/20402#comment:31
https://trac.sagemath.org/ticket/20402#comment:31
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:29" title="Comment 29">nthiery</a>:
</p>
<blockquote class="citation">
<p>
In any cases, we should keep consistency with Weyl groups. It's currently <code>w.action(l)</code> there (not <code>w.act(l)</code> btw); if we switch to <code>_act_on_</code>, then we should do it consistently everywhere.
</p>
</blockquote>
<p>
Term-wise, I think that <code>w</code> acts on <code>l</code>, while there is an action of <code>W</code> on <code>l</code>. But I am equally okay to use <code>w.action(l)</code> for consistency.
</p>
<p>
I don't get your point of switching: what is wrong with having the method <code>action</code> and then using this method in the body of <code>_act_on_</code>?
</p>
TicketgitTue, 26 Apr 2016 10:40:50 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:32
https://trac.sagemath.org/ticket/20402#comment:32
<ul>
<li><strong>commit</strong>
changed from <em>283ccf598079150e16406adc6a5a55b519f8ec26</em> to <em>295d784db0ae24bed97ed7b4d3777df9dbd652c2</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=295d784db0ae24bed97ed7b4d3777df9dbd652c2"><span class="icon"></span>295d784</a></td><td><code>replace act* by action*</code>
</td></tr></table>
TicketnthieryTue, 26 Apr 2016 10:46:54 GMT
https://trac.sagemath.org/ticket/20402#comment:33
https://trac.sagemath.org/ticket/20402#comment:33
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:31" title="Comment 31">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
I don't get your point of switching: what is wrong with having the method <code>action</code> and then using this method in the body of <code>_act_on_</code>?
</p>
</blockquote>
<p>
Just that I am not sure that we want to overload/polute '*' with this; and if we do, it should be done consistently for Weyl groups as well.
</p>
<p>
Cheers
</p>
Ticketstumpc5Tue, 26 Apr 2016 11:21:50 GMT
https://trac.sagemath.org/ticket/20402#comment:34
https://trac.sagemath.org/ticket/20402#comment:34
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:33" title="Comment 33">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:31" title="Comment 31">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
I don't get your point of switching: what is wrong with having the method <code>action</code> and then using this method in the body of <code>_act_on_</code>?
</p>
</blockquote>
<p>
Just that I am not sure that we want to overload/polute '*' with this; and if we do, it should be done consistently for Weyl groups as well.
</p>
</blockquote>
<p>
One reason in favour is that elements in <code>CoxeterGroup</code> are matrices, so they use '*' anyways. Otherwise, I would have to re-enable the method <code>action</code> there which is simply using '*'.
</p>
<p>
One reason against is that for complex reflection groups, the action on the space and on its dual are different. And since both are represented by vectors (in the dual bases {ei} and {xi}), the '*' does not see this for now. But one could have an optional argument <code>in_dual</code> for <code>action</code>.
</p>
TicketnthieryTue, 26 Apr 2016 12:43:18 GMT
https://trac.sagemath.org/ticket/20402#comment:35
https://trac.sagemath.org/ticket/20402#comment:35
<p>
Oh, I had not noticed that <code>CoxeterMatrixGroup</code> were acting on plain vectors with '*'. As you mention this is very ambiguous. For a Weyl group this is even worst: is the vector interpreted as in the span of the (co)roots? of the (co)weights? in the ambient space?
</p>
<p>
For now, I'd rather keep the notations as explicit as possible (in particular, it's easier to document a method <code>.action</code> than an overloaded operator). And leave us room in the future to overload '*', either by default, or upon explicit request from the user.
</p>
Ticketstumpc5Tue, 26 Apr 2016 15:04:15 GMT
https://trac.sagemath.org/ticket/20402#comment:36
https://trac.sagemath.org/ticket/20402#comment:36
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:35" title="Comment 35">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Oh, I had not noticed that <code>CoxeterMatrixGroup</code> were acting on plain vectors with '*'.
</p>
</blockquote>
<p>
Yes, the elements are matrices which act on vectors :-)
</p>
<blockquote class="citation">
<p>
As you mention this is very ambiguous. For a Weyl group this is even worst: is the vector interpreted as in the span of the (co)roots? of the (co)weights? in the ambient space?
</p>
</blockquote>
<p>
I agree.
</p>
<blockquote class="citation">
<p>
For now, I'd rather keep the notations as explicit as possible (in particular, it's easier to document a method <code>.action</code> than an overloaded operator). And leave us room in the future to overload '*', either by default, or upon explicit request from the user.
</p>
</blockquote>
<p>
One could define <code>_act_on_</code> in different ways for elements in the root/coroot/weight/coweight space, and not at all for plain vectors. But on the reflection group side there is no class for elements in <code>V</code> and in <code>V^*</code> yet. I am okay with both: considering the action on vectors for now on elements in <code>V</code>, or removing the coercion for now.
</p>
TickettscrimTue, 26 Apr 2016 15:16:10 GMT
https://trac.sagemath.org/ticket/20402#comment:37
https://trac.sagemath.org/ticket/20402#comment:37
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:27" title="Comment 27">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:25" title="Comment 25">tscrim</a>:
</p>
<blockquote class="citation">
<p>
For the speed issues, how much of this is making a difference?
</p>
</blockquote>
<p>
The complete implementation depends on <code>has_descents</code> and multiplication of elements to be fast (and this will get another boost for <code>ReflectionGroup</code> in <a class="closed ticket" href="https://trac.sagemath.org/ticket/20484" title="enhancement: Use the cythonized versions of has_descent, first_descent, descents ... (closed: invalid)">#20484</a>), when you <code>%prun</code> the code for <code>CoxeterGroup</code> you don't see a single bottleneck but that it is too slow in many parts (I am not complaining about this speed issue in general, this implementation is uniform for all Coxeter systems, finite or infinite, so that's great and the speed is what one gets there!)
</p>
</blockquote>
<p>
That is not answering the question I asked. I asked about having one additional level of indirection to have a specialized function for when the input is an instance of <code>RealReflectionGroup</code>.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
If it does, you can separate out the important functionality into a separate method, and then subclass the general class which special cases the methods when you use <code>ReflectionGroup</code>.
</p>
</blockquote>
<p>
This implementation of subword complexes is <strong>not</strong> a general implementation that could be moved up the hierarchy, and then a few methods could be replaced for concrete implementations. All the internal core methods are written so that internally it only plays with numbers and permutations of numbers, you only see roots popping out when using the API.
</p>
</blockquote>
<p>
I'm sure there are only some key parts of it which are written that strongly depend on the input format. These could be abstracted to work in general with a special subclass which has the specialized method or into the implementations of the Coxeter group. This is a standard design pattern for code that I've seen many times over (we even have the former in Sage's matrix code).
</p>
<blockquote class="citation">
<p>
Is is indeed possible to write a toy (in terms of speed) implementation for the category of <code>CoxeterGroups</code>, but I am not the one doing that at the moment -- what I provide is a research level implementation for <code>ReflectionGroup</code>.
</p>
</blockquote>
<p>
In many ways, we are beating a dead horse as you have basically done what I am asking for already (with the specializing being done in the different implementations of a Coxeter group).
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
However, let me be a bit more stern, I will not set this ticket to a positive review when you essentially remove doctest coverage because <code>gap3</code> is not an optional package yet (although this argument weakens once it is). Moreover, you can test all of the functionality with standard Sage, which such tests should be localized to each method, not (only) in a class-level test which does the "core" functionality.
</p>
</blockquote>
<p>
Hm -- you see my complaints about that above. On the other hand, I also see your point of getting rotten untested code after a while, and I don't really care if we pollute the documentation with more tests.
</p>
</blockquote>
<p>
Then let's leave those tests in. That is the only thing I am asking for here.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
By renaming <code>action_on_root_indices</code>, you need to deprecate it.
</p>
</blockquote>
<p>
It was only there for a few months, see <a class="closed ticket" href="https://trac.sagemath.org/ticket/11010" title="enhancement: Implementation of the SubwordComplex as defined by Knutson and Miller (closed: fixed)">#11010</a>, and this method is almost surely not used by anyone so far. You still think it should be deprecated ?
</p>
</blockquote>
<p>
I am pretty sure it appears in a stable release (i.e., 7.1), so yes.
</p>
TickettscrimTue, 26 Apr 2016 15:17:42 GMT
https://trac.sagemath.org/ticket/20402#comment:38
https://trac.sagemath.org/ticket/20402#comment:38
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:30" title="Comment 30">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:27" title="Comment 27">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
How could crystallographic-ness being different for the group and for its Coxeter type ? See Section 2.8 in Humphreys Coxeter group book for a "proof" that the two are the same.
</p>
</blockquote>
<p>
I guess that's Travis's point: mathematically, they two should be the same. If there are two methods implementing this fact, then we need to make sure that they remain consistent.
</p>
</blockquote>
<p>
Well, my interpretation of Christian's first comment was this was a property of the realization of the group, not of the group itself. So the group's <code>is_crystallographic</code> should just call that of its Coxeter type/matrix.
</p>
TickettscrimTue, 26 Apr 2016 15:19:16 GMT
https://trac.sagemath.org/ticket/20402#comment:39
https://trac.sagemath.org/ticket/20402#comment:39
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:33" title="Comment 33">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:31" title="Comment 31">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
I don't get your point of switching: what is wrong with having the method <code>action</code> and then using this method in the body of <code>_act_on_</code>?
</p>
</blockquote>
<p>
Just that I am not sure that we want to overload/polute '*' with this; and if we do, it should be done consistently for Weyl groups as well.
</p>
</blockquote>
<p>
I have been told by a few people that they would want this feature (in the finite Coxeter group setting) as it is natural and corresponds to what we would write mathematically.
</p>
TickettscrimTue, 26 Apr 2016 15:24:20 GMT
https://trac.sagemath.org/ticket/20402#comment:40
https://trac.sagemath.org/ticket/20402#comment:40
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:35" title="Comment 35">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Oh, I had not noticed that <code>CoxeterMatrixGroup</code> were acting on plain vectors with '*'. As you mention this is very ambiguous. For a Weyl group this is even worst: is the vector interpreted as in the span of the (co)roots? of the (co)weights? in the ambient space?
</p>
</blockquote>
<p>
For <code>CoxeterMatrixGroup</code>, it is given as a (faithful) representation, so the basis of a vector space is (implicitly) given, which is the roots. For the Weyl group, it is also given as a representation, so again, we have a basis which we define the action on. So I see no ambiguity, and in fact, I think the Weyl groups are very explicit about this.
</p>
<blockquote class="citation">
<p>
For now, I'd rather keep the notations as explicit as possible (in particular, it's easier to document a method <code>.action</code> than an overloaded operator). And leave us room in the future to overload '*', either by default, or upon explicit request from the user.
</p>
</blockquote>
<p>
I agree, that it is easier to document <code>action()</code> than an overloaded operator, but I think it is only marginally easier. The first thing I would (and do) look at is the class-level documentation for what I can do with a particular object. So if we have examples in there that describe the action of <code>*</code>, I think that makes it easily accessible what we are doing, in addition to the naturality of the action. Or am I forgetting something (again, in the [finite] Coxeter group setting)?
</p>
TicketnthieryTue, 26 Apr 2016 15:25:34 GMT
https://trac.sagemath.org/ticket/20402#comment:41
https://trac.sagemath.org/ticket/20402#comment:41
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:39" title="Comment 39">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I have been told by a few people that they would want this feature (in the finite Coxeter group setting) as it is natural and corresponds to what we would write mathematically.
</p>
</blockquote>
<p>
Well, mathematicaly, I tend to write <code>v.s_i</code>, not <code>vs_i</code>, precisely to raise the potential ambiguity (this is an action, not a multiplication)
</p>
Ticketstumpc5Tue, 26 Apr 2016 15:30:20 GMT
https://trac.sagemath.org/ticket/20402#comment:42
https://trac.sagemath.org/ticket/20402#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:38" title="Comment 38">tscrim</a>:
</p>
<blockquote class="citation">
<p>
In many ways, we are beating a dead horse as you have basically done what I am asking for already (with the specializing being done in the different implementations of a Coxeter group).
</p>
</blockquote>
<p>
Well, let's keep it as it is for now then.
</p>
<blockquote class="citation">
<p>
Then let's leave those tests in. That is the only thing I am asking for here.
</p>
</blockquote>
<p>
Fine.
</p>
<blockquote class="citation">
<p>
I am pretty sure it appears in a stable release (i.e., 7.1), so yes.
</p>
</blockquote>
<p>
After Nicolas' comment that <code>WeylGroup</code> uses <code>.action</code>, I moved it back to <code>action*</code>...
</p>
<blockquote class="citation">
<p>
my interpretation of Christian's first comment was this was a property of the realization of the group, not of the group itself. So the group's <code>is_crystallographic</code> should just call that of its Coxeter type/matrix.
</p>
</blockquote>
<p>
Yes, for real groups I only wanted the <code>is_crystallographic</code> of the group to call the one of the Coxeter type/matrix. But complex reflection groups can still be crystallographic (though they only are if they are real and crystallographic), so they deserve the method <code>is_crystallographic</code> without having a Coxeter type/Coxeter matrix attached to them.
</p>
<blockquote class="citation">
<p>
So I see no ambiguity, and in fact, I think the Weyl groups are very explicit about this.
</p>
</blockquote>
<p>
But what if <code>w</code> is an element of the group and <code>v</code> is a *Sage vector*. Does it represent an element in the vector space spanned the simple roots, or by the simple coroots, or by the fundamental weights, or by the fundamental coweights? Or does that not matter since the actions are all the same?
</p>
TickettscrimTue, 26 Apr 2016 15:37:14 GMT
https://trac.sagemath.org/ticket/20402#comment:43
https://trac.sagemath.org/ticket/20402#comment:43
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:42" title="Comment 42">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:38" title="Comment 38">tscrim</a>:
</p>
<blockquote class="citation">
<p>
my interpretation of Christian's first comment was this was a property of the realization of the group, not of the group itself. So the group's <code>is_crystallographic</code> should just call that of its Coxeter type/matrix.
</p>
</blockquote>
<p>
Yes, for real groups I only wanted the <code>is_crystallographic</code> of the group to call the one of the Coxeter type/matrix. But complex reflection groups can still be crystallographic (though they only are if they are real and crystallographic), so they deserve the method <code>is_crystallographic</code> without having a Coxeter type/Coxeter matrix attached to them.
</p>
</blockquote>
<p>
Ah, I see. This good.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
So I see no ambiguity, and in fact, I think the Weyl groups are very explicit about this.
</p>
</blockquote>
<p>
But what if <code>w</code> is an element of the group and <code>v</code> is a *Sage vector*. Does it represent an element in the vector space spanned the simple roots, or by the simple coroots, or by the fundamental weights, or by the fundamental coweights? Or does that not matter since the actions are all the same?
</p>
</blockquote>
<p>
Sage's vectors exists expressed in some canonical basis, and by doing matrix multiplication, there is an implicit assumption that these bases are the same. In a way, what you are advocating for is that we need to explicitly specify the matrix for any matrix <code>m</code> and <code>v</code> and check those are the same anytime we write <code>m * v</code>. The various subclasses of <code>CombinatorialFreeModule</code> does this by saying the (parent) class corresponds to a choice of basis, but this is not, and really can never be, the case for Sage's vectors, where there is an assumption of a canonical basis. In fact, because these matrices are representations of a vector space given in terms of a basis, that basis is canonical as far as Sage's vectors are concerned. (I hope that makes some sense...)
</p>
Ticketstumpc5Tue, 26 Apr 2016 16:06:19 GMT
https://trac.sagemath.org/ticket/20402#comment:44
https://trac.sagemath.org/ticket/20402#comment:44
<blockquote class="citation">
<p>
Sage's vectors exists expressed in some canonical basis, and by doing matrix multiplication, there is an implicit assumption that these bases are the same.
</p>
</blockquote>
<p>
I'd say that multiplying a matrix <code>M</code> with a vector <code>v</code> (as a tuple in distinction to vector space elements) is well-defined and completely independent on bases. So that's totally fine to do as <code>M*v</code>.
</p>
<p>
Only if the matrix is representing a linear map and the tuple represents a vector space element there is an assumption on both being represented in the same basis. When you now represent a Coxeter group element as a matrix you make a choice of basis (which might be the root basis, its dual, some linearly independent vectors of an "ambient space", or any other) This is now what you stick this matrix to, but when you do <code>w*v</code>, one does not specify the matrix that represents <code>w</code> in some basis, so this operation is ambiguous, as it didn't specify on which space <code>w</code> acts here (i.e., it is not clear in which basis you represent <code>w</code> as a matrix for that action.
</p>
<p>
For <code>ReflectionGroup</code> I just thought that the best is to have the method <code>.action</code> to take two parameters <code>side</code> being <code>"left"</code> or <code>"right"</code> and <code>on_space</code> being <code>"primal"</code> or <code>"dual"</code>.
</p>
<p>
Beside that, I am okay with providing the coercion to act on the primal, but am also okay with removing it again (I am slightly in favour of keeping it though).
</p>
TickettscrimTue, 26 Apr 2016 16:31:14 GMT
https://trac.sagemath.org/ticket/20402#comment:45
https://trac.sagemath.org/ticket/20402#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:44" title="Comment 44">stumpc5</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Sage's vectors exists expressed in some canonical basis, and by doing matrix multiplication, there is an implicit assumption that these bases are the same.
</p>
</blockquote>
<p>
I'd say that multiplying a matrix <code>M</code> with a vector <code>v</code> (as a tuple in distinction to vector space elements) is well-defined and completely independent on bases. So that's totally fine to do as <code>M*v</code>.
</p>
</blockquote>
<p>
No, that is not true except in the most abstract sense of a linear morphism and an element of a vector space. A matrix comes with an implicit choice of a basis, and when we write vectors as tuples, there is an implicit choice of a basis as well. For example, if <code>M</code> is a diagonalizable matrix acting on a vector space <code>V</code>, we can find some basis such that <code>M</code> can be expressed as acting on scalars on that basis. While the diagonal matrix and <code>M</code> are different matrices, they define the <em>same</em> linear automorphism on <code>V</code>, and hence, have the same action on a vector. Yet, to do the computation, we need to express the vector in terms of our basis.
</p>
<blockquote class="citation">
<p>
Only if the matrix is representing a linear map and the tuple represents a vector space element there is an assumption on both being represented in the same basis. When you now represent a Coxeter group element as a matrix you make a choice of basis (which might be the root basis, its dual, some linearly independent vectors of an "ambient space", or any other) This is now what you stick this matrix to, but when you do <code>w*v</code>, one does not specify the matrix that represents <code>w</code> in some basis, so this operation is ambiguous, as it didn't specify on which space <code>w</code> acts here (i.e., it is not clear in which basis you represent <code>w</code> as a matrix for that action.
</p>
</blockquote>
<p>
That is not how a <code>CoxeterMatrixGroup</code> or a <code>WeylGroup</code> is defined. It is given precisely by that choice of representation/matrix, so in a way, it is a Coxeter system along with a specified representation.
</p>
<p>
Also, any <code>v</code> (implicitly) carries with it a vector space (not up to isomorphism), and we are checking to see if the Coxeter group has a defined action on that vector space. An alternative viewpoint is if <code>v</code> is a Sage vector, in a way, we are only considering that element up to isomorphism, so we have lost some information (but again, that is if we don't fix a representation which makes a canonical choice of basis for the vector space). In contrast, for <code>v</code> being an element of the root/weight/ambient space, we have an explicit basis and we have defined where the roots are in these, so the action is well-defined and explicit.
</p>
<blockquote class="citation">
<p>
For <code>ReflectionGroup</code> I just thought that the best is to have the method <code>.action</code> to take two parameters <code>side</code> being <code>"left"</code> or <code>"right"</code> and <code>on_space</code> being <code>"primal"</code> or <code>"dual"</code>.
</p>
<p>
Beside that, I am okay with providing the coercion to act on the primal, but am also okay with removing it again (I am slightly in favour of keeping it though).
</p>
</blockquote>
<p>
In case there is any ambiguity, I'm also only really advocating for using <code>*</code> in the real reflection group setting. Moreover, I think we should avoid using Sage vectors for the roots (at least in the generic parents) because there is some ambiguity about their basis.
</p>
Ticketstumpc5Tue, 26 Apr 2016 17:31:42 GMT
https://trac.sagemath.org/ticket/20402#comment:46
https://trac.sagemath.org/ticket/20402#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:45" title="Comment 45">tscrim</a>:
</p>
<blockquote class="citation">
<p>
No, that is not true except in the most abstract sense of a linear morphism and an element of a vector space. A matrix comes with an implicit choice of a basis, and when we write vectors as tuples, there is an implicit choice of a basis as well.
</p>
</blockquote>
<p>
The point I wanted to make is that given a field k, you can define an nxn matrix M over k as a table of entries in k and you can define an n-tuple to be an n-tuple x of entries in k, and you can multiply them Mx by some explicit row-column definition. At this point, there is no basis involved. One can even enrich the set k<sup>n with a vector space structure and then see the multiplication by M at a linear map without ever talking about bases. And this multiplication is defined in Sage, even if k is not a field so there is no as simple concept of bases.
</sup></p>
<p>
Only if one wants to consider a k-linear map phi : V -> V on a vector space V over k as a matrix, then the basis comes into play.
</p>
<p>
Of course one could say that k<sup>n has an implicit basis in which the above matrix represents a linear map, but that is not quite adequate since k</sup>n comes with a canonical standard basis, so there is no choice of an implicit basis.
</p>
<blockquote class="citation">
<p>
For example, if <code>M</code> is a diagonalizable matrix acting on a vector space <code>V</code>,
</p>
</blockquote>
<p>
in your argument, you always assume a general vector space V, there I agree that you play with explicit or implicit bases.
</p>
<blockquote class="citation">
<p>
That is not how a <code>CoxeterMatrixGroup</code> or a <code>WeylGroup</code> is defined. It is given precisely by that choice of representation/matrix, so in a way, it is a Coxeter system along with a specified representation.
</p>
</blockquote>
<p>
That's right, and I am fine with arguing that this action of the group should be used when doing <code>w*v</code>. But there are still other spaces involved on which the group also acts, so if you do not specify which space <code>v</code> lives in one has to <strong>implicitly</strong> assume it to be this representation.
</p>
<blockquote class="citation">
<p>
In case there is any ambiguity, I'm also only really advocating for using <code>*</code> in the real reflection group setting.
</p>
</blockquote>
<p>
My proposition for reflection groups at the moment is
</p>
<ul><li>have a method <code>.action(vec, side="left", on_space="primal")</code> that does the appropriate action, and
</li><li>have <code>._act_on_(vec, self_on_left)</code> defined as <code>.action(vec, side=side, on_space="primal")</code> where the side is set depending on self_on_left being <code>True</code> or <code>False</code>.
</li></ul><blockquote class="citation">
<p>
Moreover, I think we should avoid using Sage vectors for the roots (at least in the generic parents) because there is some ambiguity about their basis.
</p>
</blockquote>
<p>
I agree, but I have not yet implemented root spaces and coroot spaces for crg's. So I don't have anything better to provide for now than vectors. But that's on the todo list...
</p>
Ticketstumpc5Tue, 26 Apr 2016 17:35:26 GMT
https://trac.sagemath.org/ticket/20402#comment:47
https://trac.sagemath.org/ticket/20402#comment:47
<p>
One quick question outside of finite type Weyl and Coxeter groups <code>W</code>: is it correct that the actions of <code>W</code> on <code>V</code> and on <code>V^*</code> are the same. This is, the image of (x1,...,xn) under <code>w</code> is the same for (x1,...,xn) being a vector in <code>V</code> expressed in the basis {ei} and being a vector in <code>V^*</code> expressed in the dual basis {xi}?
</p>
TickettscrimWed, 27 Apr 2016 04:56:53 GMT
https://trac.sagemath.org/ticket/20402#comment:48
https://trac.sagemath.org/ticket/20402#comment:48
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:46" title="Comment 46">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:45" title="Comment 45">tscrim</a>:
Of course one could say that k<sup>n</sup> has an implicit basis in which the above matrix represents a linear map, but that is not quite adequate since k<sup>n</sup> comes with a canonical standard basis, so there is no choice of an implicit basis.
</p>
</blockquote>
<p>
There is a standard basis, but it is not canonical.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
That is not how a <code>CoxeterMatrixGroup</code> or a <code>WeylGroup</code> is defined. It is given precisely by that choice of representation/matrix, so in a way, it is a Coxeter system along with a specified representation.
</p>
</blockquote>
<p>
That's right, and I am fine with arguing that this action of the group should be used when doing <code>w*v</code>. But there are still other spaces involved on which the group also acts, so if you do not specify which space <code>v</code> lives in one has to <strong>implicitly</strong> assume it to be this representation.
</p>
</blockquote>
<p>
This is no more implicit than the relationship with the chosen basis of a matrix and a vector. I still contend that by fixing a representation, we now have a canonical basis for <code>V</code> (and we implicitly assume <code>v</code> is an element of <code>V</code>). For good reasons, we can't tell if <code>v</code> is to be considered an element in the representation or some other (isomorphic) vector space.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
In case there is any ambiguity, I'm also only really advocating for using <code>*</code> in the real reflection group setting.
</p>
</blockquote>
<p>
My proposition for reflection groups at the moment is
</p>
<ul><li>have a method <code>.action(vec, side="left", on_space="primal")</code> that does the appropriate action, and
</li><li>have <code>._act_on_(vec, self_on_left)</code> defined as <code>.action(vec, side=side, on_space="primal")</code> where the side is set depending on self_on_left being <code>True</code> or <code>False</code>.
</li></ul></blockquote>
<p>
That is good with me.
</p>
Ticketstumpc5Wed, 27 Apr 2016 05:44:03 GMT
https://trac.sagemath.org/ticket/20402#comment:49
https://trac.sagemath.org/ticket/20402#comment:49
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:48" title="Comment 48">tscrim</a>:
</p>
<blockquote class="citation">
<p>
There is a standard basis, but it is not canonical.
</p>
</blockquote>
<p>
Okay, let's agree that we both know the situation, but that we do not agree which structure to emphasize...
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>have a method <code>.action(vec, side="left", on_space="primal")</code> that does the appropriate action, and
</li><li>have <code>._act_on_(vec, self_on_left)</code> defined as <code>.action(vec, side=side, on_space="primal")</code> where the side is set depending on self_on_left being <code>True</code> or <code>False</code>.
</li></ul></blockquote>
<p>
That is good with me.
</p>
</blockquote>
<p>
Good!
</p>
<p>
@Nicolas, do you agree this to be reasonable or do even not like this usage of the coercion?
</p>
TicketnthieryWed, 27 Apr 2016 16:11:54 GMT
https://trac.sagemath.org/ticket/20402#comment:50
https://trac.sagemath.org/ticket/20402#comment:50
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/20402#comment:49" title="Comment 49">stumpc5</a>:
</p>
<blockquote class="citation">
<p>
@Nicolas, do you agree this to be reasonable or do even not like this usage of the coercion?
</p>
</blockquote>
<p>
When in doubt, I tend to take the path that is easier to change in the
future in case one change one's mind after gaining more experience. In
this case, it's easier to add the coercion later on than to remove it,
or replace it by something else.
</p>
<p>
But that's no strong opinion. Proceed as you believe is right.
</p>
<p>
Cheers,
</p>
TicketgitFri, 29 Apr 2016 19:49:17 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:51
https://trac.sagemath.org/ticket/20402#comment:51
<ul>
<li><strong>commit</strong>
changed from <em>295d784db0ae24bed97ed7b4d3777df9dbd652c2</em> to <em>619ba6e8422ce14b34cbfed3d9f982a72cf27183</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=dd4413d10a3eac41b84bdb4ebdd1f17879b19abc"><span class="icon"></span>dd4413d</a></td><td><code>edited the reflection representation and its dual -- still to be tested</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=29842f6baa18cacfc80d5007cc87bfb058d475a4"><span class="icon"></span>29842f6</a></td><td><code>still working on the left/right primal/dual repr</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=619ba6e8422ce14b34cbfed3d9f982a72cf27183"><span class="icon"></span>619ba6e</a></td><td><code>doubled all doctests to ReflectionGroup and CoxeterGroup</code>
</td></tr></table>
Ticketstumpc5Fri, 29 Apr 2016 19:52:39 GMT
https://trac.sagemath.org/ticket/20402#comment:52
https://trac.sagemath.org/ticket/20402#comment:52
<p>
I am still struggling with the left/right action -- here, we want left actions (i.e., matrix*vector), but the standard for reflection groups is right action. I know how to turn left to right action for real and for well-generated groups, but still don't get it right to have it for the badly generated groups.
</p>
<p>
Actually, this shouldn't hold this ticket of, but I haven't resorted things yet...
</p>
TicketgitSat, 30 Apr 2016 05:39:48 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:53
https://trac.sagemath.org/ticket/20402#comment:53
<ul>
<li><strong>commit</strong>
changed from <em>619ba6e8422ce14b34cbfed3d9f982a72cf27183</em> to <em>f0353a5ff43a9c1c5b113379513350a194416dc8</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=f0353a5ff43a9c1c5b113379513350a194416dc8"><span class="icon"></span>f0353a5</a></td><td><code>working out the left/right action</code>
</td></tr></table>
TicketgitSat, 30 Apr 2016 08:06:48 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:54
https://trac.sagemath.org/ticket/20402#comment:54
<ul>
<li><strong>commit</strong>
changed from <em>f0353a5ff43a9c1c5b113379513350a194416dc8</em> to <em>5076f0bfcaae0e7592262d1bddc0df9ff5452230</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=5076f0bfcaae0e7592262d1bddc0df9ff5452230"><span class="icon"></span>5076f0b</a></td><td><code>Merge branch 'develop' into u/stumpc5/20402</code>
</td></tr></table>
Ticketstumpc5Sat, 30 Apr 2016 08:37:11 GMTdependencies changed
https://trac.sagemath.org/ticket/20402#comment:55
https://trac.sagemath.org/ticket/20402#comment:55
<ul>
<li><strong>dependencies</strong>
changed from <em>#11187</em> to <em>#11187, #20521</em>
</li>
</ul>
TicketgitSat, 30 Apr 2016 08:39:23 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:56
https://trac.sagemath.org/ticket/20402#comment:56
<ul>
<li><strong>commit</strong>
changed from <em>5076f0bfcaae0e7592262d1bddc0df9ff5452230</em> to <em>8ffeaf9449529ed0f1d05007fc16a3ae4c5143f5</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=f25f5d6f8f5f7abf157088c9429e6a6355942fe1"><span class="icon"></span>f25f5d6</a></td><td><code>take over the implementation from #20402</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9e557e23840e48d7c00524edea2f7a70e3f39573"><span class="icon"></span>9e557e2</a></td><td><code>fixing a few doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c0ebcca6a74a37dc81d2ec85c0f5ab8fb634dc51"><span class="icon"></span>c0ebcca</a></td><td><code>merged 20521</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b5a56ccd0c240d24950e9e969511ac55b3fa1910"><span class="icon"></span>b5a56cc</a></td><td><code>added indirect doctest flags</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8ffeaf9449529ed0f1d05007fc16a3ae4c5143f5"><span class="icon"></span>8ffeaf9</a></td><td><code>Merge branch 'u/stumpc5/20521' into u/stumpc5/20402</code>
</td></tr></table>
Ticketstumpc5Sat, 30 Apr 2016 08:48:48 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/20402#comment:57
https://trac.sagemath.org/ticket/20402#comment:57
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#11187, #20521</em> to <em>#20521</em>
</li>
</ul>
TicketgitSat, 30 Apr 2016 20:20:37 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:58
https://trac.sagemath.org/ticket/20402#comment:58
<ul>
<li><strong>commit</strong>
changed from <em>8ffeaf9449529ed0f1d05007fc16a3ae4c5143f5</em> to <em>03b1ae00fbf0857b4ee29e819008d01f751dbdae</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=eaa9da0dc1ca6332fda0569dfcc698c4fde0a669"><span class="icon"></span>eaa9da0</a></td><td><code>Some minor reviewer changes.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e71529efca88b4fa8c3fb7b67924333fd75041fb"><span class="icon"></span>e71529e</a></td><td><code>minor improvement of as_matrix + removal of todo note</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=03b1ae00fbf0857b4ee29e819008d01f751dbdae"><span class="icon"></span>03b1ae0</a></td><td><code>Merge branch 'u/stumpc5/20521' into u/stumpc5/20402</code>
</td></tr></table>
Ticketstumpc5Sat, 30 Apr 2016 20:25:49 GMT
https://trac.sagemath.org/ticket/20402#comment:59
https://trac.sagemath.org/ticket/20402#comment:59
<p>
I just rechecked -- beside the changes now moved to <a class="closed ticket" href="https://trac.sagemath.org/ticket/20521" title="enhancement: Left and right actions for real reflection groups, actions on the ... (closed: fixed)">#20521</a> and the few changes in <code>coxeter_groups.py</code>, there are almost entirely newly added doctests for <code>ReflectionGroup</code>.
</p>
TickettscrimSat, 30 Apr 2016 20:36:25 GMT
https://trac.sagemath.org/ticket/20402#comment:60
https://trac.sagemath.org/ticket/20402#comment:60
<p>
Instead of
</p>
<pre class="wiki">print "Caution: the Minkowski summands are build with rational vertices."
</pre><p>
(which it should be print("foo") to be Python3 compatible) you should use Sage's warning mechanism:
</p>
<pre class="wiki">from warnings import warn
warn("foo", RuntimeWarning)
</pre><p>
Otherwise LGTM.
</p>
TicketgitSat, 30 Apr 2016 20:56:44 GMTcommit changed
https://trac.sagemath.org/ticket/20402#comment:61
https://trac.sagemath.org/ticket/20402#comment:61
<ul>
<li><strong>commit</strong>
changed from <em>03b1ae00fbf0857b4ee29e819008d01f751dbdae</em> to <em>71eba72e896ba17314bcab6965f33005d02a26e4</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=71eba72e896ba17314bcab6965f33005d02a26e4"><span class="icon"></span>71eba72</a></td><td><code>using python warnings</code>
</td></tr></table>
Ticketstumpc5Sat, 30 Apr 2016 20:58:13 GMTreviewer set
https://trac.sagemath.org/ticket/20402#comment:62
https://trac.sagemath.org/ticket/20402#comment:62
<ul>
<li><strong>reviewer</strong>
set to <em>Frederic Chapoton, Travis Scrimshaw</em>
</li>
</ul>
<p>
Done, thx also here! -- more improvements to come hopefully soon...
</p>
TickettscrimSat, 30 Apr 2016 23:01:24 GMT
https://trac.sagemath.org/ticket/20402#comment:63
https://trac.sagemath.org/ticket/20402#comment:63
<p>
Frédéric, Nicolas, anything else before setting this to a positive review?
</p>
TickettscrimSat, 30 Apr 2016 23:09:14 GMTreviewer changed
https://trac.sagemath.org/ticket/20402#comment:64
https://trac.sagemath.org/ticket/20402#comment:64
<ul>
<li><strong>reviewer</strong>
changed from <em>Frederic Chapoton, Travis Scrimshaw</em> to <em>Frédéric Chapoton, Travis Scrimshaw</em>
</li>
</ul>
Ticketstumpc5Wed, 04 May 2016 11:22:23 GMT
https://trac.sagemath.org/ticket/20402#comment:65
https://trac.sagemath.org/ticket/20402#comment:65
<p>
A quick *ping* here to check whether we can get this ticket merged...
</p>
TickettscrimWed, 04 May 2016 14:58:31 GMTstatus changed
https://trac.sagemath.org/ticket/20402#comment:66
https://trac.sagemath.org/ticket/20402#comment:66
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
If there is possibly anything else, then they can set this back from a positive review.
</p>
TicketvbraunThu, 05 May 2016 09:27:15 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/20402#comment:67
https://trac.sagemath.org/ticket/20402#comment:67
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>u/stumpc5/20402</em> to <em>71eba72e896ba17314bcab6965f33005d02a26e4</em>
</li>
</ul>
Ticket