Opened 8 years ago

Closed 6 years ago

# Extended Affine Weyl Groups SD40

Reported by: Owned by: bump bump major sage-6.8 combinatorics days54, coxeter, days64, days65 sage-combinat, aschilling, nthiery, mshimo, tscrim Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry Dan Bump, Anne Schilling N/A f549344 #10963, #14102

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.

### comment:1 Changed 8 years ago by bump

• Owner changed from (none) to bump
• Type changed from PLEASE CHANGE to enhancement

### Changed 8 years ago by bump

Implements Extended Affine Iwahori Hecke Algebras

### comment:2 Changed 8 years ago by bump

• Description modified (diff)

### comment:3 Changed 8 years ago by bump

• Cc aschilling nthiery mshimo added

### comment:4 Changed 8 years ago by bump

• 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 vbraun_spam

• Milestone changed from sage-6.0 to sage-6.1

### comment:6 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:7 Changed 8 years ago by aschilling

• Branch set to public/combinat/extended_affine_weyl_groups-15375
• 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 git

• 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:10 Changed 8 years ago by git

• 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 mshimo

Doctests now pass

### comment:12 Changed 8 years ago by git

• 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 git

• Commit changed from 69b91167e2f82170992fa9c91b21105f4a33f7d5 to 90ed1b11dc51dfcb454feff3dfbe95fe56b12b75

Branch pushed to git repo; I updated commit sha1. New commits:

 ​90ed1b1 changed calling syntax

### comment:15 Changed 7 years ago by git

• 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 vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:17 Changed 7 years ago by git

• 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 git

• 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 git

• 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 vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:21 Changed 7 years ago by git

• Commit changed from 7c1ed09b81c0e3cd4e898ba0941a898fcc36f0b3 to db6924af6c99f3bfa679be505f6b1ca5d73d890d

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f3cce2f merge ​db6924a more merging

### comment:22 Changed 7 years ago by aschilling

• Status changed from new to needs_review

### comment:23 Changed 7 years ago by git

• 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 git

• 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 bump

• Reviewers set to bump

### comment:26 Changed 7 years ago by bump

• Description modified (diff)

### comment:27 Changed 7 years ago by bump

All tests in combinat/root_system pass.

### comment:28 Changed 7 years ago by bump

• Status changed from needs_review to needs_work

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.)

Last edited 7 years ago by bump (previous) (diff)

### comment:29 Changed 6 years ago by git

• 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 bump

• Status changed from needs_work to needs_review

### comment:31 Changed 6 years ago by bump

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.

Last edited 6 years ago by bump (previous) (diff)

### comment:32 Changed 6 years ago by bump

• 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"

Last edited 6 years ago by bump (previous) (diff)

### comment:33 Changed 6 years ago by git

• 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_groups-15375

### comment:34 Changed 6 years ago by bump

• Milestone changed from sage-6.4 to sage-6.7

### comment:35 Changed 6 years ago by bump

To facilitate review, here is a link to a build of documentation.

### comment:36 follow-up: ↓ 37 Changed 6 years ago by git

• 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_groups-15375

### comment:37 in reply to: ↑ 36 ; follow-up: ↓ 38 Changed 6 years ago by aschilling

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 aschilling

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 git

• 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 git

• Commit changed from 95c982263863e58b6aa07a3e10bb16add6c25642 to 461ea30fd203504555aed68fa738ed89f6fbc0e3

Branch pushed to git repo; I updated commit sha1. New commits:

 ​44e4306 Merge branch 'public/combinat/extended_affine_weyl_groups-15375' into 6.8.b1 ​461ea30 trac #15375 adding import of Infinity

### comment:41 Changed 6 years ago by git

• Commit changed from 461ea30fd203504555aed68fa738ed89f6fbc0e3 to ea3d56c10c1291520b4dbfd8c59e365203e5a00d

Branch pushed to git repo; I updated commit sha1. New commits:

 ​5d57171 Merge branch 'public/combinat/extended_affine_weyl_groups-15375' into 6.8.b2 ​ea3d56c trac #15375 remove duplicate Kac reference

### comment:42 Changed 6 years ago by git

• 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 git

• Commit changed from 6ee5258434bfeb7d083ffd0f96f47d1ef41376b6 to 3835fa9c70161d4765ce76583b2825591bf13888

Branch pushed to git repo; I updated commit sha1. New commits:

 ​3835fa9 trac #15375 forgot two old-style raise

### comment:44 follow-up: ↓ 45 Changed 6 years ago by git

• 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 aschilling

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 git

• 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 git

Branch pushed to git repo; I updated commit sha1. New commits:

 ​3dab47a 15375: undid changes in categories/weyl_group.py

### comment:48 follow-up: ↓ 49 Changed 6 years ago by git

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 aschilling

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:51 Changed 6 years ago by git

• Commit changed from 018bf969db55cb76c017ead141f99c22e4cabc25 to afc40d7638cdcf20b45119c6ed86205919eb65a9

Branch pushed to git repo; I updated commit sha1. New commits:

 ​dfa6037 doc fixes ​afc40d7 Merge branch 'public/combinat/extended_affine_weyl_groups-15375' of trac.sagemath.org:sage into extendedJune9

### comment:52 Changed 6 years ago by mshimo

• 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 follow-up: ↓ 54 Changed 6 years ago by git

• 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_groups-15375 ​d146bdd Merge branch 'public/combinat/extended_affine_weyl_groups-15375' of git://trac.sagemath.org/sage into public/combinat/extended_affine_weyl_groups-15375

### comment:54 in reply to: ↑ 53 Changed 6 years ago by aschilling

Hi Mark,

I rebased your patch on top of sage-6.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 demazure-lusztig 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)
<ipython-input-28-d05fc29a74b6> in <module>()
----> 1 E.inject_shorthands()

/Applications/sage/local/lib/python2.7/site-packages/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)
<ipython-input-44-d96974f8367c> in <module>()
----> 1 E.group_generators()

/Applications/sage/local/lib/python2.7/site-packages/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 aschilling

• Milestone changed from sage-6.7 to sage-6.8
• Reviewers changed from bump to Dan Bump, Anne Schilling
• Status changed from needs_review to needs_work

### comment:56 follow-up: ↓ 57 Changed 6 years ago by mshimo

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 aschilling

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 bump

I'll have a look.

### comment:59 Changed 6 years ago by git

• 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 git

• 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 mshimo

The various realizations should now have a generators method.

### comment:62 Changed 6 years ago by git

• 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 follow-up: ↓ 64 Changed 6 years ago by mshimo

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 aschilling

Hi Mark,

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 git

• 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 follow-up: ↓ 69 Changed 6 years ago by mshimo

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 mshimo

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 git

• 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 aschilling

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)
<ipython-input-3-d69de239d91b> in <module>()
----> 1 PW0.group_generators()

/Applications/sage/local/lib/python2.7/site-packages/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 follow-up: ↓ 71 Changed 6 years ago by 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".

### comment:71 in reply to: ↑ 70 Changed 6 years ago by aschilling

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 git

• Commit changed from c021d03217b1f205a577e0a9a1a7f6b1d567789b to b9152e2bc08cd314744b5fd5ef627a467778f25a

Branch pushed to git repo; I updated commit sha1. New commits:

 ​addcb09 fixed group_generators methods ​b9152e2 Merge branch 'public/combinat/extended_affine_weyl_groups-15375' of trac.sagemath.org:sage into extended

### comment:73 follow-ups: ↓ 74 ↓ 75 Changed 6 years ago by mshimo

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 aschilling

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 nthiery

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 nthiery

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 type-free, 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 by cartan_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 to reduced_words?
• om -> omega (I believe that's what we use in the crystal code)

in general, please avoid abbreviations, unless they correspond to standard one-letter mathematical notations.

• ..., finite = True) -> ..., finite=True)
• The code is currently assuming that the 0 is the "main" special node, right? Please use cartan_type.special_node() instead.
• What about fusing the methods family and group_generators? group_generators should return a family anyway (unlike gens).
• 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).
Version 0, edited 6 years ago by nthiery (next)

### comment:77 follow-up: ↓ 78 Changed 6 years ago by mshimo

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 nthiery

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 git

• 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 mshimo

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 mshimo

I mean  __iter__; the formatting lost the double underscores.

### comment:82 Changed 6 years ago by nthiery

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!

Last edited 6 years ago by nthiery (previous) (diff)

### comment:83 Changed 6 years ago by git

• 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 git

• 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 mshimo

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 bump

Nicolas, are you satisfied? (Can the status be changed back to needs_review?)

### comment:87 Changed 6 years ago by git

• 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 git

• 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 git

• 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 mshimo

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 git

• 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 git

Branch pushed to git repo; I updated commit sha1. New commits:

 ​2b78b7b Merge branch 'develop' into public/combinat/extended_affine_weyl_groups-15375

### comment:93 Changed 6 years ago by aschilling

• Status changed from needs_work to needs_review

### comment:94 Changed 6 years ago by nthiery

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 use sage.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 use return 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 for E 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? or classical_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 like from_classical or make this an element method to_affine? Same for the related methods.
• cardinality is already provided by Sets().Infinite()
• group_generators this could in principle go in Groups().WithRealizations(). If creating the latter just for this seems like overkill, you can just add a comment about this.
• ...Realizations.ParentMethods.one: replacing PW0 by a_realization the code would be generic and could go in Magmas.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: maybe representation? 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 tscrim

For Cartan types, it is simply classical and for crystals, it is classical_object (ex. classical_weight).

### comment:96 follow-up: ↓ 111 Changed 6 years ago by bump

I looked at the patchbot shortlogs for the first three machines reporting build failure. They all report the same error:

OSError: [combinat ] /home/worker/sage-patchbot/local/lib/python2.7/site-packages/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:
Expected:
Got:
**********************************************************************


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 bump

• Status changed from needs_review to needs_work

### comment:98 Changed 6 years ago by git

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_groups-15375' into extended

### comment:99 Changed 6 years ago by tscrim

The one() method added to unital magmas will need a doctest.

### comment:100 Changed 6 years ago by mshimo

• Branch changed from public/combinat/extended_affine_weyl_groups-15375 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_groups-15375' into extended

### comment:101 Changed 6 years ago by mshimo

• Branch changed from ba64e29 to u/mshimo/combinat/extended_affine_weyl_groups-15375
• 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 follow-up: ↓ 103 Changed 6 years ago by mshimo

Ugh. I clearly don't know how to use trac.

### comment:103 in reply to: ↑ 102 Changed 6 years ago by aschilling

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
**********************************************************************
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 bump

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 git

• 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 mshimo

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 bump

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



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.

Last edited 6 years ago by bump (previous) (diff)

### comment:108 follow-up: ↓ 109 Changed 6 years ago by 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.

### comment:109 in reply to: ↑ 108 Changed 6 years ago by aschilling

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> --set-upstream-to 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 mshimo

• Branch changed from u/mshimo/combinat/extended_affine_weyl_groups-15375 to public/15375

### comment:111 in reply to: ↑ 96 ; follow-up: ↓ 114 Changed 6 years ago by nthiery

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:
Expected:
Got:
**********************************************************************


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 git

• 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 mshimo

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 bump

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 git

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 mshimo

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 follow-up: ↓ 118 Changed 6 years ago by bump

• 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 aschilling

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/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/Applications/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/sage/interfaces/gap.py", line 568, in eval
result = Expect.eval(self, input_line, **kwds)
File "/Applications/sage/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1239, in eval
for L in code.split('\n') if L != ''])
File "/Applications/sage/local/lib/python2.7/site-packages/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/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/Applications/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/sage/interfaces/gap.py", line 568, in eval
result = Expect.eval(self, input_line, **kwds)
File "/Applications/sage/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1239, in eval
for L in code.split('\n') if L != ''])
File "/Applications/sage/local/lib/python2.7/site-packages/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);
**********************************************************************
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 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 tscrim

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.

Last edited 6 years ago by tscrim (previous) (diff)

### comment:120 follow-up: ↓ 121 Changed 6 years ago by bump

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 aschilling

Thanks Travis, that did the trick. For me all tests pass now.

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 sage-6.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 bump

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 aschilling

• Status changed from needs_review to positive_review

### comment:124 Changed 6 years ago by vbraun

• 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 mshimo

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 jdemeyer

Now that this ticket needs_work anyway:

Currently, two of the files have no copyright statement and two do not state the GPL version.

### comment:127 Changed 6 years ago by bump

Here is a log of the results of sage --docbuild reference pdf

(Temporary URL)

### comment:128 Changed 6 years ago by jhpalmieri

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 def ExtendedAffineWeylGroup(cartan_type, general_linear=None, **print_options): There are isomorphisms .. MATH:: :nowrap: \begin{align*} 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 class GroupSemidirectProduct(CartesianProduct): .. 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} If act_to_right is False, the group G \rtimes H is specified by a homomorphism \psi\in \mathrm{Hom}(H,\mathrm{Aut}(G)) such that class GroupSemidirectProduct(CartesianProduct): .. MATH:: \begin{align*} \begin{aligned} (g_1,h_1)(g_2,h_2) &= g_1 h_1 g_2 h_2 \\ &= g_1 h_1 g_2 h_1^{-1} h_1 h_2 \\ &= (g_1 twist(h_1,g_2), h_1 h_2) \end{align*} \end{aligned} If prefix0 (resp. prefixl) is not None then it is used as a wrapper for printing elements of G (resp. H). If print_tuple is True then elements are printed

### comment:129 Changed 6 years ago by bump

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 mshimo

Thanks, Dan, that is very helpful. I will fix the doc.

### comment:131 Changed 6 years ago by tscrim

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 git

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f549344 fixed doc errors

### comment:133 Changed 6 years ago by bump

• 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 vbraun

• 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 jdemeyer

• Authors changed from Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas Thiery. to Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry
• Commit f54934489e6183a647a9195d277a1a9344ac831b deleted

### comment:136 Changed 6 years ago by kcrisman

• Authors changed from Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry to Dan Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry

### comment:137 Changed 5 years ago by chapoton

• Authors changed from Dan Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry to Daniel Bump, Dan Orr, Anne Schilling, Mark Shimozono, Nicolas M. Thiéry
Note: See TracTickets for help on using tickets.