#15375 closed enhancement (fixed)
Extended Affine Weyl Groups SD40
Reported by:  bump  Owned by:  bump 

Priority:  major  Milestone:  sage6.8 
Component:  combinatorics  Keywords:  days54, coxeter, days64, days65 
Cc:  sagecombinat, aschilling, nthiery, mshimo, tscrim  Merged in:  
Authors:  Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry  Reviewers:  Dan Bump, Anne Schilling 
Report Upstream:  N/A  Work issues:  
Branch:  f549344 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #10963, #14102  Stopgaps: 
Description (last modified by )
The first version of this patch implementing Affine Weyl Groups was written during Sage Days 40. The original contributers were Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas Thiery. Subsequently Mark Shimozono added multiple realizations and twisted types.
Attachments (1)
Change History (138)
comment:1 Changed 8 years ago by
 Owner changed from (none) to bump
 Type changed from PLEASE CHANGE to enhancement
Changed 8 years ago by
comment:2 Changed 8 years ago by
 Description modified (diff)
comment:3 Changed 8 years ago by
 Cc aschilling nthiery mshimo added
comment:4 Changed 8 years ago by
 Description modified (diff)
 Summary changed from Extended Affine Iwahori Hecke Algebras SD40 to Extended Affine Weyl Groups SD40
comment:5 Changed 8 years ago by
 Milestone changed from sage6.0 to sage6.1
comment:6 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:7 Changed 8 years ago by
 Branch set to public/combinat/extended_affine_weyl_groups15375
 Commit set to 6d9dd29abed89e4b0217e644ba34f16902efbf4f
New commits:
0251a33  Trac #13394: Implement faster and safer WeakValueDictionary

9d9cae3  # Sat Oct 19 11:50:04 2013 +0200

b2914f3  # Sun Oct 27 13:58:49 2013 +0100

362fd5e  # Tue Oct 29 20:14:19 2013 +0100

dfc1fe8  #14102: Nonsymmetric Macdonald polynomials

4c44e62  imported patch extended_affine_weyl_groups_sd40.patch

6d9dd29  fixed extended affine Weyl groups

comment:8 Changed 8 years ago by
 Commit changed from 6d9dd29abed89e4b0217e644ba34f16902efbf4f to 6e725643031d4780bad4483db626bca7ae553e01
Branch pushed to git repo; I updated commit sha1. New commits:
6e72564  added prefix support for extended affine Weyl groups

comment:9 Changed 8 years ago by
 Cc tscrim added
comment:10 Changed 8 years ago by
 Commit changed from 6e725643031d4780bad4483db626bca7ae553e01 to e293db988bcda8694701c35d7dff4b6e5dfc525f
Branch pushed to git repo; I updated commit sha1. New commits:
e293db9  added dual form of translation lattices

comment:11 Changed 8 years ago by
Doctests now pass
comment:12 Changed 8 years ago by
 Commit changed from e293db988bcda8694701c35d7dff4b6e5dfc525f to 69b91167e2f82170992fa9c91b21105f4a33f7d5
Branch pushed to git repo; I updated commit sha1. New commits:
69b9116  fixed bug in type BCdual

comment:13 Changed 7 years ago by
 Commit changed from 69b91167e2f82170992fa9c91b21105f4a33f7d5 to 90ed1b11dc51dfcb454feff3dfbe95fe56b12b75
Branch pushed to git repo; I updated commit sha1. New commits:
90ed1b1  changed calling syntax

comment:14 Changed 7 years ago by
 Keywords coxeter added
comment:15 Changed 7 years ago by
 Commit changed from 90ed1b11dc51dfcb454feff3dfbe95fe56b12b75 to fb8a6e096d087e8b0c7dd91e81717b9c33bde242
Branch pushed to git repo; I updated commit sha1. New commits:
fb8a6e0  fund gp special nodes returns tuple

comment:16 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:17 Changed 7 years ago by
 Commit changed from fb8a6e096d087e8b0c7dd91e81717b9c33bde242 to d534a0d3b583dc905ad02d962fb64d58cf5defb4
Branch pushed to git repo; I updated commit sha1. New commits:
d534a0d  extended affine weyl group for GL

comment:18 Changed 7 years ago by
 Commit changed from d534a0d3b583dc905ad02d962fb64d58cf5defb4 to dc20ae933c3354b2279446bd8df0d00283f00e80
Branch pushed to git repo; I updated commit sha1. New commits:
dc20ae9  fixed looking for memory leak

comment:19 Changed 7 years ago by
 Commit changed from dc20ae933c3354b2279446bd8df0d00283f00e80 to 7c1ed09b81c0e3cd4e898ba0941a898fcc36f0b3
Branch pushed to git repo; I updated commit sha1. New commits:
7c1ed09  initial merge of extended affine weyl group with gl

comment:20 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:21 Changed 7 years ago by
 Commit changed from 7c1ed09b81c0e3cd4e898ba0941a898fcc36f0b3 to db6924af6c99f3bfa679be505f6b1ca5d73d890d
comment:22 Changed 7 years ago by
 Keywords days64 added
 Status changed from new to needs_review
comment:23 Changed 7 years ago by
 Commit changed from db6924af6c99f3bfa679be505f6b1ca5d73d890d to 6708533d8357f38f656c758c53af6c7fb410def7
Branch pushed to git repo; I updated commit sha1. New commits:
6708533  removed FundamentalGroup etc from global name space

comment:24 Changed 7 years ago by
 Commit changed from 6708533d8357f38f656c758c53af6c7fb410def7 to 66e07a2bb515487a80d8ad494c4ef286d8a181cc
Branch pushed to git repo; I updated commit sha1. New commits:
66e07a2  changes to documentation

comment:25 Changed 7 years ago by
 Reviewers set to bump
comment:26 Changed 7 years ago by
 Description modified (diff)
comment:27 Changed 7 years ago by
All tests in combinat/root_system pass.
comment:28 Changed 7 years ago by
 Status changed from needs_review to needs_work
We should add a line
sage/combinat/root_system/extended_affine_weyl_group
to doc/en/reference/combinat/module_list.rst
to get the documentation
into the reference manual. However if we do this we get an error:
OSError: [combinat ] extended_affine_weyl_group.py:docstring of ExtendedAffineWeylGroup:11: ERROR: The "TOPIC" directive may not be used within topics or body elements.
I don't find TOPIC used very much in the sage documentation. I do find it used in combinat/root_system/plot.py, which is a standalone tutorial. Maybe it can't be used in a docstring. I hoped that changing TOPIC to RUBRIC would work, but I tried it and it doesn't.
I'm leaving the status "needs work" since the documentation needs to compile.
(Maybe the problem is the double colon after TOPIC. I don't have time to try that now.)
comment:29 Changed 6 years ago by
 Commit changed from 66e07a2bb515487a80d8ad494c4ef286d8a181cc to 8ab8135351102f87e07da9a5b347a5643a9fb0a1
Branch pushed to git repo; I updated commit sha1. New commits:
8ab8135  doc cleanup

comment:30 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:31 Changed 6 years ago by
I removed the double colon after TOPIC so that the documentation builds. I also added extended_affine_weyl_group to reference/combinat/module_list and combinat/root_system/init.py.
As far as I know the patch is good now and I've changed the status back to needs review.
comment:32 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
Since some docstrings don't have doctests I'm changing the status back to "needs work"
comment:33 Changed 6 years ago by
 Commit changed from 8ab8135351102f87e07da9a5b347a5643a9fb0a1 to 737ca2190d7b681df1706bb81af7f605fb3f7383
Branch pushed to git repo; I updated commit sha1. New commits:
737ca21  Merge branch 'develop' into public/combinat/extended_affine_weyl_groups15375

comment:34 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.7
comment:35 Changed 6 years ago by
To facilitate review, here is a link to a build of documentation.
http://sporadic.stanford.edu/reference1/combinat/sage/combinat/root_system/extended_affine_weyl_group.html
comment:36 followup: ↓ 37 Changed 6 years ago by
 Commit changed from 737ca2190d7b681df1706bb81af7f605fb3f7383 to ecfe8e2d164710fdf9957be6c47588cfb9d73cbc
Branch pushed to git repo; I updated commit sha1. New commits:
ecfe8e2  Merge branch 'develop' into public/combinat/extended_affine_weyl_groups15375

comment:37 in reply to: ↑ 36 ; followup: ↓ 38 Changed 6 years ago by
I rebased on the latest development version and will look what else needs to be done to finalize this patch!
Anne
comment:38 in reply to: ↑ 37 Changed 6 years ago by
Here are some comments on the patch. Mark, can you work on them so we can review them?
 There are several methods in
ExtendedAffineWeylGroup_Class
that do not have documentation or tests.
 I remember that we talked about this at Sage Days 64: can we get rid of
to_ambient
in all the various places?
fundamental_groups.py
also does not have full documentation coverage!
 Also
pyflakes extended_affine_weyl_group.py extended_affine_weyl_group.py:940: undefined name 'Infinity'
 Many methods in
sage.groups.group_semidirect_product.py
have no documentation.
Best,
Anne
comment:39 Changed 6 years ago by
 Commit changed from ecfe8e2d164710fdf9957be6c47588cfb9d73cbc to 95c982263863e58b6aa07a3e10bb16add6c25642
Branch pushed to git repo; I updated commit sha1. New commits:
95c9822  removed some unused imports

comment:40 Changed 6 years ago by
 Commit changed from 95c982263863e58b6aa07a3e10bb16add6c25642 to 461ea30fd203504555aed68fa738ed89f6fbc0e3
comment:41 Changed 6 years ago by
 Commit changed from 461ea30fd203504555aed68fa738ed89f6fbc0e3 to ea3d56c10c1291520b4dbfd8c59e365203e5a00d
comment:42 Changed 6 years ago by
 Commit changed from ea3d56c10c1291520b4dbfd8c59e365203e5a00d to 6ee5258434bfeb7d083ffd0f96f47d1ef41376b6
Branch pushed to git repo; I updated commit sha1. New commits:
6ee5258  trac #15385 py3 form of raise statements

comment:43 Changed 6 years ago by
 Commit changed from 6ee5258434bfeb7d083ffd0f96f47d1ef41376b6 to 3835fa9c70161d4765ce76583b2825591bf13888
Branch pushed to git repo; I updated commit sha1. New commits:
3835fa9  trac #15375 forgot two oldstyle raise

comment:44 followup: ↓ 45 Changed 6 years ago by
 Commit changed from 3835fa9c70161d4765ce76583b2825591bf13888 to d4a052e65c7d950509cb3588d66b5e249c6cca3d
Branch pushed to git repo; I updated commit sha1. New commits:
1ea8239  18634 fixed

634a452  Merge branch 'extended' into newextended

058c82f  Merge branch 'newextended' into extendedJune8

00668e1  added doctests for full coverage

886b52a  put reflection_to_root and reflection_to_coroot into parent methods

52eac7e  Merge branch 'fix_reflection_to_root' into extendedJune8

4a28b14  Merge branch 'extendedJune8' into extendedJune9

d4a052e  completed doctest coverage

comment:45 in reply to: ↑ 44 Changed 6 years ago by
Hi Mark,
I see that reflection_to_root
is changed in this branch. Did you rebase it on top of http://trac.sagemath.org/ticket/18634
?
Anne
comment:46 Changed 6 years ago by
 Commit changed from d4a052e65c7d950509cb3588d66b5e249c6cca3d to 309c327b244483b650cf7c2ad7a2dc1facf708c0
Branch pushed to git repo; I updated commit sha1. New commits:
309c327  undid the 18635 stuff

comment:47 Changed 6 years ago by
 Commit changed from 309c327b244483b650cf7c2ad7a2dc1facf708c0 to 3dab47ad8b0cbac670d035848d2bfd68fd5bb175
Branch pushed to git repo; I updated commit sha1. New commits:
3dab47a  15375: undid changes in categories/weyl_group.py

comment:48 followup: ↓ 49 Changed 6 years ago by
 Commit changed from 3dab47ad8b0cbac670d035848d2bfd68fd5bb175 to 018bf969db55cb76c017ead141f99c22e4cabc25
Branch pushed to git repo; I updated commit sha1. New commits:
018bf96  15375: some more doc fixes

comment:49 in reply to: ↑ 48 Changed 6 years ago by
Please add what the INPUT is in the following methods in the extended_affine_weyl_group:
apply_simple_projection coset_representative face_data action translation_group_morphism simple_reflection bruhat_le
There are certain methods that occur multiple times with code that looks only
minimally different (for example in has_descent
it looks like that only left and
right have changed). Could you explain why one needs the methods in each realization
separately rather than having one general one.
Same in src/sage/groups/group_exp.py. There are methods where some of the input needs to be explained.
comment:50 Changed 6 years ago by
 Keywords days65 added
comment:51 Changed 6 years ago by
 Commit changed from 018bf969db55cb76c017ead141f99c22e4cabc25 to afc40d7638cdcf20b45119c6ed86205919eb65a9
comment:52 Changed 6 years ago by
 Status changed from needs_work to needs_review
Since certain methods are much more efficiently computed in certain realizations, the generic implementation just consists of converting to the favorable realization, doing the computation there using a custom implementation, and then converting back. But since converting is often expensive, sometimes a direct algorithm is given for more than one realization. This is the case for :meth:has_descent.
comment:53 followup: ↓ 54 Changed 6 years ago by
 Commit changed from afc40d7638cdcf20b45119c6ed86205919eb65a9 to d146bdd5c5c5d1fbeace1b039c15d63d085cdba4
Branch pushed to git repo; I updated commit sha1. New commits:
c2efddb  Merge branch 'develop' into public/combinat/extended_affine_weyl_groups15375

d146bdd  Merge branch 'public/combinat/extended_affine_weyl_groups15375' of git://trac.sagemath.org/sage into public/combinat/extended_affine_weyl_groups15375

comment:54 in reply to: ↑ 53 Changed 6 years ago by
Hi Mark,
I rebased your patch on top of sage6.8.beta4. So please pull the changes before you keep editing!
I see this in the diff without a doc test and a comment:
diff git a/src/sage/combinat/root_system/type_affine.py b/src/sage/combinat/root_system/type_affine.py index f505c47..c19136e 100644  a/src/sage/combinat/root_system/type_affine.py +++ b/src/sage/combinat/root_system/type_affine.py @@ 419,6 +419,14 @@ class AmbientSpace(CombinatorialFreeModule): return vector(list(vector(classical._plot_projection(classical(x)))) + [x["deltacheck"]]) + def from_vector_notation(self, weight, style="lattice"): + """ + ..TODO:: CHECK THIS STUPID DEFAULT IMPLEMENTATION WITH DAN + + This is in particular used to compute demazurelusztig characters + """ + return weight
Can this be taken out?
When I tried the following, I got an error message
sage: E.inject_shorthands()  NotImplementedError Traceback (most recent call last) <ipythoninput28d05fc29a74b6> in <module>() > 1 E.inject_shorthands() /Applications/sage/local/lib/python2.7/sitepackages/sage/categories/sets_cat.pyc in inject_shorthands(self, verbose) 2351 from sage.misc.misc import inject_variable 2352 if not hasattr(self, "_shorthands"): > 2353 raise NotImplementedError("no shorthands defined for {}".format(self)) 2354 for shorthand in self._shorthands: 2355 realization = getattr(self, shorthand)() NotImplementedError: no shorthands defined for Extended affine Weyl group of type ['A', 2, 1]
Do you want shorthands for all the realizations?
Similarly,
sage: E.group_generators()  NotImplementedError Traceback (most recent call last) <ipythoninput44d96974f8367c> in <module>() > 1 E.group_generators() /Applications/sage/local/lib/python2.7/sitepackages/sage/categories/groups.pyc in group_generators(self) 108 return Family(self.gens()) 109 except AttributeError: > 110 raise NotImplementedError("no generators are implemented for this group") 111 112 def monoid_generators(self): NotImplementedError: no generators are implemented for this group
Best,
Anne
comment:55 Changed 6 years ago by
 Milestone changed from sage6.7 to sage6.8
 Reviewers changed from bump to Dan Bump, Anne Schilling
 Status changed from needs_review to needs_work
comment:56 followup: ↓ 57 Changed 6 years ago by
The rebasing weirdness is coming from someone else's code.
For shorthands, the realizations are created by the methods PW0(), W0P(), WF(), FW(), PvW0(), and W0Pv(). Um, do you think this is insufficiently short?
Implementing generators is easy and I can add that.
comment:57 in reply to: ↑ 56 Changed 6 years ago by
Replying to mshimo:
The rebasing weirdness is coming from someone else's code.
You mean the method from_vector_notation? It looks like it belongs to this patch, but it might be super old. Should we take it out?
For shorthands, the realizations are created by the methods PW0(), W0P(), WF(), FW(), PvW0(), and W0Pv(). Um, do you think this is insufficiently short?
Well, I just tried out methods that E.<tab> offered and many of them are not implemented.
Implementing generators is easy and I can add that.
That would be good!
Dan, could you also do a final review of this patch? I think it is getting close to being ready.
comment:58 Changed 6 years ago by
I'll have a look.
comment:59 Changed 6 years ago by
 Commit changed from d146bdd5c5c5d1fbeace1b039c15d63d085cdba4 to 870af046ea18ee9530da67918157777807e5b425
Branch pushed to git repo; I updated commit sha1. New commits:
870af04  15375: small change

comment:60 Changed 6 years ago by
 Commit changed from 870af046ea18ee9530da67918157777807e5b425 to cb2c7d78ab90a474c4cd320719c101cbbd14b95d
Branch pushed to git repo; I updated commit sha1. New commits:
cb2c7d7  added generators methods

comment:61 Changed 6 years ago by
The various realizations should now have a generators method.
comment:62 Changed 6 years ago by
 Commit changed from cb2c7d78ab90a474c4cd320719c101cbbd14b95d to 9e25def4bd0a2d9b5ad4e654d57d637880da9f7d
Branch pushed to git repo; I updated commit sha1. New commits:
9e25def  implemented list method for fundamental group

comment:63 followup: ↓ 64 Changed 6 years ago by
I added a list method for the fundamental groups. I am not sure that I did this entirely correctly (that is, implementing the correct part of the category framework). I just added a method and added finiteness information to the category.
comment:64 in reply to: ↑ 63 Changed 6 years ago by
Hi Mark,
Does this address my original question about
sage: E=ExtendedAffineWeylGroup(['A',2,1]) sage: E.group_generators()
or are you fixing something else?
Shall we remove from_vector_notation
?
Best,
Anne
comment:65 Changed 6 years ago by
 Commit changed from 9e25def4bd0a2d9b5ad4e654d57d637880da9f7d to 4438e0950c8e0758ab961813259a66650fc2b35a
Branch pushed to git repo; I updated commit sha1. New commits:
4438e09  fixed bugs related to finiteness of fundamental group

comment:66 followup: ↓ 69 Changed 6 years ago by
Get rid of from_vector_notation.
Group generators are not relevant for E. E is not an actual realization.
E.PW0() is a group and so on.
The list method is related to finiteness (saying "x in G" makes sense if G is a finite group, and iterating over x in G uses the list method). I somehow got on the wrong branch and unfixed some stuff and had to refix it.
comment:67 Changed 6 years ago by
As far as E.group_generators() is concerned, one could call the default realization of E and invoke its "generators" method.
comment:68 Changed 6 years ago by
 Commit changed from 4438e0950c8e0758ab961813259a66650fc2b35a to c021d03217b1f205a577e0a9a1a7f6b1d567789b
Branch pushed to git repo; I updated commit sha1. New commits:
c021d03  15375: removed from_vector_notation

comment:69 in reply to: ↑ 66 Changed 6 years ago by
Replying to mshimo:
Get rid of from_vector_notation.
I got rid of it in my last commit.
Group generators are not relevant for E. E is not an actual realization.
E.PW0() is a group and so on.
But even for that it does not work for me:
sage: E=ExtendedAffineWeylGroup(['A',2,1]) sage: PW0=E.PW0() sage: PW0.group_generators()  NotImplementedError Traceback (most recent call last) <ipythoninput3d69de239d91b> in <module>() > 1 PW0.group_generators() /Applications/sage/local/lib/python2.7/sitepackages/sage/categories/groups.pyc in group_generators(self) 108 return Family(self.gens()) 109 except AttributeError: > 110 raise NotImplementedError("no generators are implemented for this group") 111 112 def monoid_generators(self): NotImplementedError: no generators are implemented for this group
comment:70 followup: ↓ 71 Changed 6 years ago by
Sorry, I added a method "generators" not "group_generators". I think I did this to be consistent with the lattices (such as weight lattices), which use "gens" not "group_generators".
comment:71 in reply to: ↑ 70 Changed 6 years ago by
Replying to mshimo:
Sorry, I added a method "generators" not "group_generators". I think I did this to be consistent with the lattices (such as weight lattices), which use "gens" not "group_generators".
I would suggest to be consistent with what the category code asks for (in this case group_generators) since this method appears when one hits tab.completion! Otherwise it is confusing to the user.
Other than this, I will leave the final review to Dan!
comment:72 Changed 6 years ago by
 Commit changed from c021d03217b1f205a577e0a9a1a7f6b1d567789b to b9152e2bc08cd314744b5fd5ef627a467778f25a
comment:73 followups: ↓ 74 ↓ 75 Changed 6 years ago by
I fixed the group_generators methods. Note that I had to mess with sage.categories.finite_groups.
comment:74 in reply to: ↑ 73 Changed 6 years ago by
Replying to mshimo:
I fixed the group_generators methods. Note that I had to mess with sage.categories.finite_groups.
Ok, I am satisfied. Dan, could you please do a final review?
comment:75 in reply to: ↑ 73 Changed 6 years ago by
Replying to mshimo:
Note that I had to mess with sage.categories.finite_groups.
I double checked on this, and it is actually a cleanup. If this does not break tests elsewhere, I am happy with that change. Thanks!
comment:76 Changed 6 years ago by
Hi Mark,
I had a look at the fundamental group code. Thanks for the hard work!
Suggestions:
 Would it be possible to make the code more typefree, in particular the initialization? For example, it seems likely to break the current version if one uses a relabeled type BC.
 The logic for computing the special nodes would fit more naturally
in Cartan types rather than here. Please implement that as a method
cartan_type.special_nodes()
.
The special nodes form an orbit under the action of the automorphism group of the Dynkin diagram, right? If yes, then it would be enough to implement a generic method
CartanType_affine.special_nodes
, which would start from the special node as specified bycartan_type.special_node()
, and take its orbit under the automorphism group of the Dynkin diagram. It's a small graph, so that's cheap enough.
 Maybe add a test that the Cartan type is indeed affine?
 Is the
_element_constructor_
needed at all?
lambda i: finite_action_dict[i]
>finite_action_dict.__getitem__
 What about renaming
finite_action_dict
toreduced_words
?
om
>omega
(I believe that's what we use in the crystal code)
in general, please avoid abbreviations, unless they correspond to standard oneletter mathematical notations.
..., finite = True)
>..., finite=True)
 The code is currently assuming that the
0
is the "main" special node, right? Please usecartan_type.special_node()
instead.
 What about fusing the methods
family
andgroup_generators
?group_generators
should return a family anyway (unlikegens
).
 If you put the group in
XXX.FinitelyGenerated()
, the list of elements will be recovered from the group generators (of course, here we have a more straightforward way of doing it, so that may, or not, be how we want to do this).
comment:77 followup: ↓ 78 Changed 6 years ago by
Nicolas, Thanks very much for your comments.
I need to know the reason behind using cartan_type.special_node()
.
Let's call this the extra special node. For simplicity we assume untwisted affine type. By deletion we obtain a subsystem of finite type. If the extra special node is nonzero then the correct behavior is that the "classical subsystem" should be a relabeling of the appropriate standard classical subsystem with 0 as one of the "finite" Dynkin nodes. This nonstandard classical Dynkin node set will get propagated to indices of simple roots and fundamental weights and to the finite Weyl groups. In particular we could see t_{\omega_0^{\vee} as a nontrivial translation element and s_0 as an element of the finite Weyl group. Do we really want this? }
Mark
comment:78 in reply to: ↑ 77 Changed 6 years ago by
Replying to mshimo:
Nicolas, Thanks very much for your comments.
You are welcome!
I need to know the reason behind using
cartan_type.special_node()
.Let's call this the extra special node. For simplicity we assume untwisted affine type. By deletion we obtain a subsystem of finite type. If the extra special node is nonzero then the correct behavior is that the "classical subsystem" should be a relabeling of the appropriate standard classical subsystem with 0 as one of the "finite" Dynkin nodes. This nonstandard classical Dynkin node set will get propagated to indices of simple roots and fundamental weights and to the finite Weyl groups. In particular we could see t_{\omega_0^{\vee} as a nontrivial translation element and s_0 as an element of the finite Weyl group. Do we really want this? }
It's indeed nice to have 0 by default for the special node in the canonical labeling of the Dynkin diagram, in order to follow the usual conventions in the literature.
But other than that the code has been designed to be agnostic w.r.t. the labels of the nodes of the Dynkin diagram. This gives the user the flexibility to use his preferred labeling, without imposing a given convention. Also it makes the code robust in potential use cases like the following:
 The user inputs some Cartan matrix which turns out to be affine. There is no reason a priori that 0 will be a special node. (note: at some point we will need/want to implement the logic to recover some special node).
 We have a big KM Dynkin diagram, and we want to do some calculations involving a parabolic subgroup. Typically the corresponding Dynkin diagram is disconnected, and the calculation will boil down to the connected pieces. Those connected pieces could be affine Dynkin diagrams, but with labels like 3,4,5; still we might want to use it's fundamental group, etc ...
Cheers,
Nicolas
comment:79 Changed 6 years ago by
 Commit changed from b9152e2bc08cd314744b5fd5ef627a467778f25a to f36f4aad0d2d1b38ed0499b19572623c3302deec
Branch pushed to git repo; I updated commit sha1. New commits:
f36f4aa  made fixes suggested by Nicolas; added __iter__ method to fundamental group

comment:80 Changed 6 years ago by
I added an iter method because without it, doing things like [x for x in F]
was not working. I also made F an EnumeratedSet? when it is finite.
comment:81 Changed 6 years ago by
I mean __iter__
; the formatting lost the double underscores.
comment:82 Changed 6 years ago by
Good point. To avoid having both __iter__
and __list__
you can just do:
def __iter__(self): return iter(self.group_generators())
Note the use of iter
rather than for ... yield
(should be faster).
Also, default list method will already raise an error for you if the set is in XXX.Infinite()
.
Thanks!
comment:83 Changed 6 years ago by
 Commit changed from f36f4aad0d2d1b38ed0499b19572623c3302deec to e7aedfce6156862428c5410f82d43f2b8fc4ccde
Branch pushed to git repo; I updated commit sha1. New commits:
e7aedfc  fixed __iter__ method

comment:84 Changed 6 years ago by
 Commit changed from e7aedfce6156862428c5410f82d43f2b8fc4ccde to 93f5adf284de8e73ef973a792a81b11c6d113692
Branch pushed to git repo; I updated commit sha1. New commits:
93f5adf  removed is_finite method for extended affine Weyl groups and added Finite() and Infinite() information to category

comment:85 Changed 6 years ago by
I added infinite and finite information to the categories of the extended affine Weyl groups and their fundamental groups. Thanks, Nicolas, for the programming help!
comment:86 Changed 6 years ago by
Nicolas, are you satisfied? (Can the status be changed back to needs_review?)
comment:87 Changed 6 years ago by
 Commit changed from 93f5adf284de8e73ef973a792a81b11c6d113692 to fd2aa163cf1f5959fa611fec8c131c588e5cfeec
Branch pushed to git repo; I updated commit sha1. New commits:
fd2aa16  little changes

comment:88 Changed 6 years ago by
 Commit changed from fd2aa163cf1f5959fa611fec8c131c588e5cfeec to 154656f35ca2fd18f80884117ceeef31fbabfd5d
Branch pushed to git repo; I updated commit sha1. New commits:
154656f  little changes

comment:89 Changed 6 years ago by
 Commit changed from 154656f35ca2fd18f80884117ceeef31fbabfd5d to fee79e7857d9381d9d820022b39cc218d3b185cd
Branch pushed to git repo; I updated commit sha1. New commits:
fee79e7  changed fundamental_group an_element() to give a nonidentity element when possible; had to fix the doctests

comment:90 Changed 6 years ago by
I think I am done messing with this ticket for now. I added an explicit an_element method; the default one always gives the identity, which leads to less interesting example elements for doctests. So I did this and fixed the doctests.
comment:91 Changed 6 years ago by
 Commit changed from fee79e7857d9381d9d820022b39cc218d3b185cd to cece2fbe6b3623231dce2cbcd28be8f561f782a4
Branch pushed to git repo; I updated commit sha1. New commits:
cece2fb  little fix to family method of GL fundamental group and added a group_generators method for it

comment:92 Changed 6 years ago by
 Commit changed from cece2fbe6b3623231dce2cbcd28be8f561f782a4 to 2b78b7bdc2db6835dadfad978e5f6992f0546649
Branch pushed to git repo; I updated commit sha1. New commits:
2b78b7b  Merge branch 'develop' into public/combinat/extended_affine_weyl_groups15375

comment:93 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:94 Changed 6 years ago by
Hi Mark!
I have glanced half way through the patch. It's a lot of hard work you did; thanks! I'll try to go through the over half soon, but won't have the time for a systematic review, so please someone!
Here are some thoughts that popped up; take them as they just are: random food for thoughts. If something does not sound applicable, just ignore it.
weyl_groups.py
line 420,453: any reason to remove the test for s2s1s2s1?RootSystem(XXX.cartan_type())
>XXX.cartan_type().root_system()
def to_ambient_space_morphism
: you can usesage.misc.c3_controlled.identity
. Yeah, there is no reason why the identity function is defined in this module; but at least if we all use the same identity function it will be easier to refactor later on.
Variants, if you want a morphism:
End(self).identity()
or `sage: self.coerce_map_from(self)`,
..warning
>.. WARNING
 In
to_weight_space
: instead of going through a vector, one can use.sum_of_terms([i,self.scalar(alpha[i])] for ...
This method is generic and would better be in
WeightSpaceRealizations
.
to_ambient
special_nodes
: any reason not to usereturn autos.orbit(self.special_node())
?
Nicolas Thiery
>Nicolas M. Thiery
for consistency
 Doc of
ExtendedAffineWeylGroup
: use.. MATH
for the math formulas on their own line.
... the following lattices:
; idem forE
below.
 When natural, put the
::
at the end of the sentence.
 Line folding could be more consistent
 There are still quite a few of
ifs
depending on twisted/untwisted/GL; any chance to make this more generic?
For example, to define the special translation covector can't we generically use something like `c[special_node] * _special_root.associated_coroot()`? I never remember from the top of my head whether it's
c
or one of its friends that should be used here, but we did things like this in the MacDo? code.
Note that at this point the initialization of
ExtendedAffineWeylGroup_class
will probably break on a relabelled type BC.
 Do we need the
special_nodes
method on the group and the fundamental group? Or could just have it on the cartan type?
 Strip new lines just before and just after the end of the documentation string.
classical_weyl
>classical
? orclassical_weyl_group
? I don't remember what we did in similar situations for crystals and the like, but please check for consistency.
classical_weyl_to_affine_morphism
: the name is misleading since the result is not a morphism. Could we have a name likefrom_classical
or make this an element methodto_affine
? Same for the related methods.
cardinality
is already provided bySets().Infinite()
group_generators
this could in principle go inGroups().WithRealizations()
. If creating the latter just for this seems like overkill, you can just add a comment about this.
...Realizations.ParentMethods.one
: replacingPW0
bya_realization
the code would be generic and could go inMagmas.Unital.Realizations
.
Returns
>Return
coset_representative, is_grassmanian, ...
: this is the same logic as for a usual coxeter group, right? Could we avoid the duplication by having a common super category?
get_the_index
>leading_support
?
FundamentalGroup...one
:0
>special_node
an_element
: you can probably use.last()
(haven't tested)
action
: mayberepresentation
? Elsewhere in Sage,action
is usually a method that implements the action itself.
root_lattice_realizations
:Algebras
is now imported twice.
Cheers,
Nicolas
comment:95 Changed 6 years ago by
For Cartan types, it is simply classical
and for crystals, it is classical_object
(ex. classical_weight
).
comment:96 followup: ↓ 111 Changed 6 years ago by
I looked at the patchbot shortlogs for the first three machines reporting build failure. They all report the same error:
OSError: [combinat ] /home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/combinat/root_system/extended_affine_weyl_group.py:docstring of sage.combinat.root_system.extended_affine_weyl_group.ExtendedAffineWeylGroup_Class.WF_to_PW0_func:11: ERROR: Content block expected for the "WARNING" directive; none found.
I think the "Since this is used ..." at line 1068 needs to be indented. Other bots are reporting a doctest error in dev_tools.py.
********************************************************************** File "src/sage/misc/dev_tools.py", line 459, in sage.misc.dev_tools.import_statements Failed example: load_submodules(sage.sets) Expected: load sage.sets.cartesian_product... succeeded load sage.sets.set_from_iterator... succeeded Got: load sage.sets.real_set... succeeded load sage.sets.set_from_iterator... succeeded **********************************************************************
I suspect this is unrelated to this patch. It may be caused by #18553 which was merged three weeks ago and which touched sage.sets.real_set.py.
However I think the above docstring needs to be fixed and since Nicolas has some comments I'm changing the status back to needs_work.
comment:97 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:98 Changed 6 years ago by
 Commit changed from 2b78b7bdc2db6835dadfad978e5f6992f0546649 to 74c7d9edc80c6438d184979f0b06e6980b4ee196
Branch pushed to git repo; I updated commit sha1. New commits:
c0c7c0f  somehow bugfix of trac 18634 was undone. I redid it.

a06f404  some changes Nicolas suggested

79d4fe5  fixing doc strings and implementing more suggestions from Nicolas

ba64e29  more little fixes

74c7d9e  Merge branch 't/15375/public/combinat/extended_affine_weyl_groups15375' into extended

comment:99 Changed 6 years ago by
The one()
method added to unital magmas will need a doctest.
comment:100 Changed 6 years ago by
 Branch changed from public/combinat/extended_affine_weyl_groups15375 to ba64e29
 Commit 74c7d9edc80c6438d184979f0b06e6980b4ee196 deleted
It wouldn't let me push the fixed version so I made a copy of the existing ticket branch and merged it into my working branch. The result is broken. I am going to try to change the trac branch to a good one.
New commits:
c0c7c0f  somehow bugfix of trac 18634 was undone. I redid it.

a06f404  some changes Nicolas suggested

79d4fe5  fixing doc strings and implementing more suggestions from Nicolas

ba64e29  more little fixes

74c7d9e  Merge branch 't/15375/public/combinat/extended_affine_weyl_groups15375' into extended

comment:101 Changed 6 years ago by
 Branch changed from ba64e29 to u/mshimo/combinat/extended_affine_weyl_groups15375
 Commit set to c36eb6bd081b55b3785294ccff9c496e592707eb
Last 10 new commits:
93f5adf  removed is_finite method for extended affine Weyl groups and added Finite() and Infinite() information to category

fd2aa16  little changes

154656f  little changes

fee79e7  changed fundamental_group an_element() to give a nonidentity element when possible; had to fix the doctests

cece2fb  little fix to family method of GL fundamental group and added a group_generators method for it

c0c7c0f  somehow bugfix of trac 18634 was undone. I redid it.

a06f404  some changes Nicolas suggested

79d4fe5  fixing doc strings and implementing more suggestions from Nicolas

ba64e29  more little fixes

c36eb6b  added doctest to magma

comment:102 followup: ↓ 103 Changed 6 years ago by
Ugh. I clearly don't know how to use trac.
comment:103 in reply to: ↑ 102 Changed 6 years ago by
Replying to mshimo:
Ugh. I clearly don't know how to use trac.
What happened? It might happen that one cannot push if there is a change on the server that one has not pulled yet. Otherwise I would not know why this happens.
I get the following failure in sage/categories
sage t realizations.py ********************************************************************** File "realizations.py", line 44, in sage.categories.realizations.RealizationsCategory Failed example: Groups().Realizations().super_categories() Expected: [Category of groups, Category of realizations of magmas] Got: [Category of groups, Category of realizations of unital magmas] ********************************************************************** File "realizations.py", line 68, in sage.categories.realizations.Realizations Failed example: Algebras(QQ).Realizations() Expected: Join of Category of algebras over Rational Field and Category of realizations of magmas Got: Join of Category of algebras over Rational Field and Category of realizations of unital magmas ********************************************************************** 2 items had failures: 1 of 7 in sage.categories.realizations.Realizations 1 of 5 in sage.categories.realizations.RealizationsCategory [27 tests, 2 failures, 0.10 s]
comment:104 Changed 6 years ago by
There are currently doctests failures in categories/realizations.py and categories/examples/with_realizations.py. These would be fixed by correcting the doctest, viz. replacing 'magmas' by 'unital magmas' in the failing docutests.
There is also a doctest failure in misc/dev_tools.py. In comment:96 I speculated that this was not caused by this patch, but evidently this was wrong. I don't know what caused this doctest to change, but I checked out the git commit 05875d563029eb7231d2cae4b434b99e4742fbbf and confirm that dev_tools tests pass for that version. Currently this patch is rebased atop that commit and I think this proves that it caused the failure. I don't know the mechanism but the test in question seems to be pretty fragile.
There is also a build failure patchbot report (as before) that a WARNING directive is missing a content block. It looks to me as if Mark's changes should have fixed this (the content block is now indented). So I'm puzzled by this (unless the build attempt predated Mark's commit. It is date stamped 6/20.)
comment:105 Changed 6 years ago by
 Commit changed from c36eb6bd081b55b3785294ccff9c496e592707eb to dde0d4e57f9473f99c8428f378c8a1fd46c23325
Branch pushed to git repo; I updated commit sha1. New commits:
dde0d4e  fixed magma doctests

comment:106 Changed 6 years ago by
Nicolas, aren't you the author of dev_tools? Do you know what is going on with the doctest?
comment:107 Changed 6 years ago by
I noticed that the dev_tools doctest that fails is fragile: if you call it from the command line it gives a different result than the doctest.
sage: from sage.misc.dev_tools import load_submodules sage: load_submodules(sage.sets) load sage.sets.cartesian_product... succeeded load sage.sets.real_set... succeeded load sage.sets.set_from_iterator... succeeded
Presumably this is because when the doctest is run, sage.sets.real_set has already been loaded.
Since the branch was recently changed to u/mshimo I think Mark is the only one who can make changes now. When it was u/public others could push changes, for example when Anne rebased it.
comment:108 followup: ↓ 109 Changed 6 years ago by
This is part of my misunderstanding of trac.
I had trouble pushing to the u/public branch; it was forcing a merge which was creating weird errors that didn't happen in my local branch. That is why I made a new copy and put it in a new location, apparently unwittingly making it nonpublic.
I would like to just clobber (replace) the old u/public branch with the other one. Can I do that? Is it a bad idea? I just know that my previous attempts to push to that branch, created weird nonrecognizable errors that were not my code's fault.
The alternative is to make a new public branch.
comment:109 in reply to: ↑ 108 Changed 6 years ago by
Replying to mshimo:
This is part of my misunderstanding of trac.
I had trouble pushing to the u/public branch; it was forcing a merge which was creating weird errors that didn't happen in my local branch. That is why I made a new copy and put it in a new location, apparently unwittingly making it nonpublic.
I would like to just clobber (replace) the old u/public branch with the other one. Can I do that? Is it a bad idea? I just know that my previous attempts to push to that branch, created weird nonrecognizable errors that were not my code's fault.
The alternative is to make a new public branch.
You should be able to push your local branch to a public one by saying
git branch <localname> setupstreamto origin/<newremotename>
where <newremotename> is something like public/15375 (pick a name different from the old public name just in case). Then you can copy the new branch name in the trac field on the ticket.
comment:110 Changed 6 years ago by
 Branch changed from u/mshimo/combinat/extended_affine_weyl_groups15375 to public/15375
comment:111 in reply to: ↑ 96 ; followup: ↓ 114 Changed 6 years ago by
Replying to bump:
Other bots are reporting a doctest error in dev_tools.py.
********************************************************************** File "src/sage/misc/dev_tools.py", line 459, in sage.misc.dev_tools.import_statements Failed example: load_submodules(sage.sets) Expected: load sage.sets.cartesian_product... succeeded load sage.sets.set_from_iterator... succeeded Got: load sage.sets.real_set... succeeded load sage.sets.set_from_iterator... succeeded **********************************************************************I suspect this is unrelated to this patch. It may be caused by #18553 which was merged three weeks ago and which touched sage.sets.real_set.py.
My current bet is that this ticket somehow forces
sage.sets.cartesian_product
to be loaded upon Sage's startup. Hence,
when running load_submodules(sage.sets)
it does not appear anymore
as the first entry, and instead the output starts with the next one
which turns out to be sage.sets.real_set
.
If that's confirmed, a good approach would be to make sure that all the affine Weyl group code is lazy imported. That's a good property to have anyway.
Cheers,
Nicolas
comment:112 Changed 6 years ago by
 Commit changed from dde0d4e57f9473f99c8428f378c8a1fd46c23325 to e0943139b6b741cd5e4030ae99a25e2f28ccaed9
Branch pushed to git repo; I updated commit sha1. New commits:
e094313  made extended affine weyl group into a lazy import

comment:113 Changed 6 years ago by
The new module sage.categories.groups.group_semidirect_product does import CartesianProduct?. Would that make a difference?
New commits:
e094313  made extended affine weyl group into a lazy import

comment:114 in reply to: ↑ 111 Changed 6 years ago by
Replying to nthiery:
If that's confirmed, a good approach would be to make sure that all the affine Weyl group code is lazy imported. That's a good property to have anyway.
Mark's last commit did change to lazy import and did not fix the doctest.
My current bet is that this ticket somehow forces
sage.sets.cartesian_product
to be loaded upon Sage's startup.
I think this may not have to do with what is loaded on startup, but rather what is loaded during other tests coming earlier in the docstring. If this test is moved to the top of the docstring, it will give a different result. See comment:107.
comment:115 Changed 6 years ago by
 Commit changed from e0943139b6b741cd5e4030ae99a25e2f28ccaed9 to ee155e445baa2c406162df421ad8ad9b10d4292d
Branch pushed to git repo; I updated commit sha1. New commits:
ee155e4  just changed doctest in load_submodules to give the "right" answer

comment:116 Changed 6 years ago by
As you can see from the commit message, I just changed the answer in the doctest to the answer generated by the current version of sage.
comment:117 followup: ↓ 118 Changed 6 years ago by
 Status changed from needs_work to needs_review
On my machine it builds and all tests pass. But two buildbots are failing to build with cython errors.
Surely this can have nothing to do with this patch.
I'm changing the status to needs_review. I hesitate to change it to positive_review until the patchbot is happy.
comment:118 in reply to: ↑ 117 Changed 6 years ago by
Replying to bump:
On my machine it builds and all tests pass. But two buildbots are failing to build with cython errors.
Surely this can have nothing to do with this patch.
I'm changing the status to needs_review. I hesitate to change it to positive_review until the patchbot is happy.
For me everything builds fine. I ran sage testall
and got the following failures
sage t src/sage/groups/perm_gps/permgroup_named.py ********************************************************************** File "src/sage/groups/perm_gps/permgroup_named.py", line 2609, in sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_hurwitz_curve Failed example: G.ramification_module_decomposition_hurwitz_curve() # random, optional  database_gap Exception raised: Traceback (most recent call last): File "/Applications/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 496, in _run self.compile_and_execute(example, compiler, test.globs) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 858, in compile_and_execute exec(compiled, globs) File "<doctest sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_hurwitz_curve[1]>", line 1, in <module> G.ramification_module_decomposition_hurwitz_curve() # random, optional  database_gap File "/Applications/sage/local/lib/python2.7/sitepackages/sage/groups/perm_gps/permgroup_named.py", line 2630, in ramification_module_decomposition_hurwitz_curve mults = gap.eval("ram_module_hurwitz("+str(q)+")") File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/gap.py", line 568, in eval result = Expect.eval(self, input_line, **kwds) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/expect.py", line 1239, in eval for L in code.split('\n') if L != '']) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/gap.py", line 766, in _eval_line raise RuntimeError(message) RuntimeError: Gap produced error output Error, sorry, the GAP Tables Of Marks Library is not installed executing ram_module_hurwitz(13); ********************************************************************** File "src/sage/groups/perm_gps/permgroup_named.py", line 2657, in sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_modular_curve Failed example: G.ramification_module_decomposition_modular_curve() # random, optional  database_gap Exception raised: Traceback (most recent call last): File "/Applications/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 496, in _run self.compile_and_execute(example, compiler, test.globs) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 858, in compile_and_execute exec(compiled, globs) File "<doctest sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_modular_curve[1]>", line 1, in <module> G.ramification_module_decomposition_modular_curve() # random, optional  database_gap File "/Applications/sage/local/lib/python2.7/sitepackages/sage/groups/perm_gps/permgroup_named.py", line 2674, in ramification_module_decomposition_modular_curve mults = gap.eval("ram_module_X("+str(q)+")") File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/gap.py", line 568, in eval result = Expect.eval(self, input_line, **kwds) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/expect.py", line 1239, in eval for L in code.split('\n') if L != '']) File "/Applications/sage/local/lib/python2.7/sitepackages/sage/interfaces/gap.py", line 766, in _eval_line raise RuntimeError(message) RuntimeError: Gap produced error output Error, sorry, the GAP Tables Of Marks Library is not installed executing ram_module_X(7); ********************************************************************** 2 items had failures: 1 of 3 in sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_hurwitz_curve 1 of 3 in sage.groups.perm_gps.permgroup_named.PSL.ramification_module_decomposition_modular_curve [439 tests, 2 failures, 3.16 s]  sage t src/sage/groups/perm_gps/permgroup_named.py # 2 doctests failed  Total time for all tests: 3.3 seconds cpu time: 2.3 seconds cumulative wall time: 3.2 seconds
sage t src/sage/tests/gap_packages.py ********************************************************************** File "src/sage/tests/gap_packages.py", line 15, in sage.tests.gap_packages Failed example: test_packages(['atlasrep', 'tomlib']) # optional  database_gap Expected: Status Package GAP Output ++++ atlasrep true tomlib true Got: Status Package GAP Output ++++ Failure atlasrep fail Failure tomlib fail ********************************************************************** 1 item had failures: 1 of 4 in sage.tests.gap_packages [9 tests, 1 failure, 0.18 s]  sage t src/sage/tests/gap_packages.py # 1 doctest failed  Total time for all tests: 0.2 seconds cpu time: 0.2 seconds cumulative wall time: 0.2 seconds
I do not think they are related to this branch. So unless there is something else, I guess we can set positive review.
comment:119 Changed 6 years ago by
In decreasing probability, it looks like your database_gap
optional spkg is out of date, mis/not installed, or needs optional GAP packages installed directly into GAP. I'd try running sage i database_gap
and sage i gap_packages
and then running those tests again to see if they work.
comment:120 followup: ↓ 121 Changed 6 years ago by
Any conjectures about the patchbot?
I presume that the patchbot merges the patch on top of the develop tip then tries to build. Is that correct? It looks from the log as if it actually tries to merge it with a branch called patchbot/base. There doesn't actually seem to be such a branch in the repository.
comment:121 in reply to: ↑ 120 Changed 6 years ago by
Thanks Travis, that did the trick. For me all tests pass now.
Replying to bump:
Any conjectures about the patchbot?
I presume that the patchbot merges the patch on top of the develop tip then tries to build. Is that correct? It looks from the log as if it actually tries to merge it with a branch called patchbot/base. There doesn't actually seem to be such a branch in the repository.
On my local computer, I merged in the latest development version sage6.8.beta6 and everything merged fine. So I think something is wrong with the patchbot.
I am going to set positive review, since we all seem happy with the patch.
comment:122 Changed 6 years ago by
Right, I ran all the tests yesterday after a distclean build and there were no failures.
I am going to set positive review, since we all seem happy with the patch.
comment:123 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:124 Changed 6 years ago by
 Status changed from positive_review to needs_work
l.165775 \end{gather} \paragraph{Fundamental group of affine Dynkin automorph... ? ! Emergency stop. \math@cr@@@ ...@ \@ne \add@amps \maxfields@ \omit \kern \alignsep@ \iftag@ ... l.165775 \end{gather} \paragraph{Fundamental group of affine Dynkin automorph... ! ==> Fatal error occurred, no output PDF file produced! Transcript written on combinat.log. Makefile:55: recipe for target 'combinat.pdf' failed make[2]: *** [combinat.pdf] Error 1
comment:125 Changed 6 years ago by
The build of pdf documentation fails. It outputs algebra.pdf and then seems to hang. I have no idea what is going on. I didn't see a log file generated for this.
comment:126 Changed 6 years ago by
Now that this ticket needs_work anyway:
In the 4 files that you're adding in this ticket: please use the standard copyright statement as explained in http://doc.sagemath.org/html/en/developer/coding_basics.html#headingsofsagelibrarycodefiles
Currently, two of the files have no copyright statement and two do not state the GPL version.
comment:127 Changed 6 years ago by
Here is a log of the results of `sage docbuild reference pdf`
http://match.stanford.edu/pdflog
(Temporary URL)
comment:128 Changed 6 years ago by
By default, a .. MATH::
block is automatically enclosed in a math environment, so you shouldn't use an environment like align*
there: it gets turned into something like
\begin{gather} \begin{align*} ... \end{align*} \end{gather}
which is not valid LaTeX. To avoid the default, you can add :nowrap:
at the beginning of the math block, or you can use an environment like aligned
instead. This will fix it, but I would instead choose only one of these two approaches (instead of switching between them, as I've done here):

src/sage/combinat/root_system/extended_affine_weyl_group.py
diff git a/src/sage/combinat/root_system/extended_affine_weyl_group.py b/src/sage/combinat/root_system/extended_affine_weyl_group.py index 7d35cac..56d9e6d 100644
a b def ExtendedAffineWeylGroup(cartan_type, general_linear=None, **print_options): 155 155 There are isomorphisms 156 156 157 157 .. MATH:: 158 :nowrap: 158 159 159 160 \begin{align*} 160 161 W &\cong M \rtimes W_0 \cong W_0 \ltimes M \\ 
src/sage/groups/group_semidirect_product.py
diff git a/src/sage/groups/group_semidirect_product.py b/src/sage/groups/group_semidirect_product.py index 627056f..63b8769 100644
a b class GroupSemidirectProduct(CartesianProduct): 152 152 153 153 .. MATH:: 154 154 155 \begin{align *}155 \begin{aligned} 156 156 (g_1,h_1)(g_2,h_2) &= g_1 h_1 g_2 h_2 \\ 157 157 &= g_1 g_2 g_2^{1} h_1 g_2 h_2 \\ 158 158 &= (g_1g_2, twist(g_2^{1}, h_1) h_2) 159 \end{align *}159 \end{aligned} 160 160 161 161 If ``act_to_right`` is False, the group `G \rtimes H` is specified by a homomorphism `\psi\in \mathrm{Hom}(H,\mathrm{Aut}(G))` 162 162 such that … … class GroupSemidirectProduct(CartesianProduct): 175 175 176 176 .. MATH:: 177 177 178 \begin{align *}178 \begin{aligned} 179 179 (g_1,h_1)(g_2,h_2) &= g_1 h_1 g_2 h_2 \\ 180 180 &= g_1 h_1 g_2 h_1^{1} h_1 h_2 \\ 181 181 &= (g_1 twist(h_1,g_2), h_1 h_2) 182 \end{align *}182 \end{aligned} 183 183 184 184 If ``prefix0`` (resp. ``prefixl``) is not None then it is used as a wrapper for 185 185 printing elements of ``G`` (resp. ``H``). If ``print_tuple`` is True then elements are printed
comment:129 Changed 6 years ago by
There is a huge file called `doc/output/latex/en/reference/combinat/` and at line 165376 we have the following:
There are isomorphisms \begin{gather} \begin{split}\begin{align*} W &\cong M \rtimes W_0 \cong W_0 \ltimes M \\ E &\cong L \rtimes W_0 \cong W_0 \ltimes L \end{align*}\end{split}\notag \end{gather}\paragraph{Fundamental group of affine Dynkin automorphisms}
At this point pdflatex barfs.
comment:130 Changed 6 years ago by
Thanks, Dan, that is very helpful. I will fix the doc.
comment:131 Changed 6 years ago by
As John had mentioned, you need to change:
 \begin{align*} + \begin{aligned} W &\cong M \rtimes W_0 \cong W_0 \ltimes M \\ E &\cong L \rtimes W_0 \cong W_0 \ltimes L  \end{align*} + \begin{aligned}
and
.. MATH::  \begin{align*} + \begin{aligned} (g_1,h_1)(g_2,h_2) &= g_1 h_1 g_2 h_2 \\ &= g_1 g_2 g_2^{1} h_1 g_2 h_2 \\ &= (g_1g_2, twist(g_2^{1}, h_1) h_2)  \end{align*} + \end{aligned}
Also I believe this needs to be changed (there are 2 of theses):
.. MATH::  g h g^{1} &= \phi(g)(h). + g h g^{1} = \phi(g)(h).
comment:132 Changed 6 years ago by
 Commit changed from ee155e445baa2c406162df421ad8ad9b10d4292d to f54934489e6183a647a9195d277a1a9344ac831b
Branch pushed to git repo; I updated commit sha1. New commits:
f549344  fixed doc errors

comment:133 Changed 6 years ago by
 Status changed from needs_work to positive_review
Sage now builds, reference.pdf builds and all tests pass. I'm setting status back to positive review.
comment:134 Changed 6 years ago by
 Branch changed from public/15375 to f54934489e6183a647a9195d277a1a9344ac831b
 Resolution set to fixed
 Status changed from positive_review to closed
comment:135 Changed 6 years ago by
 Commit f54934489e6183a647a9195d277a1a9344ac831b deleted
Implements Extended Affine Iwahori Hecke Algebras