Opened 7 years ago
Closed 7 years ago
#15384 closed enhancement (fixed)
Improvements to root systems
Reported by:  tscrim  Owned by:  sagecombinat 

Priority:  major  Milestone:  sage6.3 
Component:  combinatorics  Keywords:  root systems, days54 
Cc:  sagecombinat, aschilling, nthiery, darij, bsalisbury1, bump  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Daniel Bump 
Report Upstream:  N/A  Work issues:  
Branch:  7285940 (Commits, GitHub, GitLab)  Commit:  72859401196f2a2893043c555146ce56102acabf 
Dependencies:  Stopgaps: 
Description (last modified by )
Implement various things for affine root systems:
 The symmetric form
(  )
.  Methods for checking the short/long/imaginary roots.
 Modelling the positive roots roots in affine types.
 Basic untwisted types (so type A_{2n}^{(2)} would give A_{2n}).
 (Dual) Coxeter numbers for finite types.
Change History (41)
comment:1 Changed 7 years ago by
 Branch set to public/combinat/root_systems/improvements15384
 Commit set to 0fa598a6ae59bc8a336e93140998a8db0e26a081
comment:2 Changed 7 years ago by
 Keywords days54 added
comment:3 Changed 7 years ago by
 Commit changed from 0fa598a6ae59bc8a336e93140998a8db0e26a081 to e40f89241889ecdc2d7f5fc8ad2d623f75584bc3
comment:4 Changed 7 years ago by
 Commit changed from e40f89241889ecdc2d7f5fc8ad2d623f75584bc3 to b4c7865c9dff6241915b6491c5f097486ee052df
Branch pushed to git repo; I updated commit sha1. New commits:
b4c7865  Merge branch 'master' into public/combinat/root_systems/improvements15384 
comment:5 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:6 Changed 7 years ago by
 Commit changed from b4c7865c9dff6241915b6491c5f097486ee052df to dd976c77935b077ef7917c413e1357b9254801f5
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
6100a4b  Merge branch 'public/combinat/root_systems/improvements15384' of trac.sagemath.org:sage into public/combinat/root_systems/improvements15384

a5a6b1c  Merge branch 'develop' into public/combinat/root_systems/improvements15384

f802ae1  Merge branch 'develop' into public/combinat/root_systems/improvements15384

d83982c  Merge branch 'develop' into public/combinat/root_systems/improvements15384

ecf8f76  Merge branch 'develop' into public/combinat/root_systems/improvements15384

c52545c  Merge branch 'develop' into public/combinat/root_systems/improvements15384

14c367e  Merge branch 'develop' into public/combinat/root_systems/improvements15384

fe68bf6  Merge branch 'develop' into public/combinat/root_systems/improvements15384

896e682  Merge branch 'develop' into public/combinat/root_systems/improvements15384

dd976c7  Merge branch 'develop' into public/combinat/root_systems/improvements15384

comment:7 Changed 7 years ago by
 Commit changed from dd976c77935b077ef7917c413e1357b9254801f5 to 2c7f793553819baad0d054082c69e029edd42e98
Branch pushed to git repo; I updated commit sha1. New commits:
2c7f793  Merge branch 'develop' into public/combinat/root_systems/improvements15384

comment:8 Changed 7 years ago by
 Commit changed from 2c7f793553819baad0d054082c69e029edd42e98 to 122ca6eca844a35e402b30cbbf71efcc6afb1e2a
Branch pushed to git repo; I updated commit sha1. New commits:
ea1641f  Merge branch 'develop' into public/combinat/root_systems/improvements15384

054b37c  Merge branch 'develop' into public/combinat/root_systems/improvements15384

30de9e8  Merge branch 'develop' into public/combinat/root_systems/improvements15384

122ca6e  Merge branch 'develop' into public/combinat/root_systems/improvements15384

comment:9 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:10 Changed 7 years ago by
 Commit changed from 122ca6eca844a35e402b30cbbf71efcc6afb1e2a to 64ac7a57e4b41d533e270b875d883cd7d910dae3
comment:11 Changed 7 years ago by
 Reviewers set to bump
comment:12 Changed 7 years ago by
 Commit changed from 64ac7a57e4b41d533e270b875d883cd7d910dae3 to 1817877be05246ac3248a6b8fc82a159dc27351c
Branch pushed to git repo; I updated commit sha1. New commits:
1817877  Almost there.

comment:13 Changed 7 years ago by
 Commit changed from 1817877be05246ac3248a6b8fc82a159dc27351c to 18fd0946874e2e54a75cdcd761311dea3f2810a4
Branch pushed to git repo; I updated commit sha1. New commits:
18fd094  Fixed affine weight lattice symmetric form.

comment:14 Changed 7 years ago by
 Reviewers changed from bump to Dan Bump
 Status changed from new to needs_review
Hey Dan,
I've fixed it. The (first r
) columns are given by P.simple_roots()
, and it's the inverse matrix should have the Kac labels (which it does).
comment:15 Changed 7 years ago by
Glad you got it fixed. I'll start looking at the patch.
comment:16 Changed 7 years ago by
Travis: do you mind updating the description of the ticket to give an overview of the changes?
Thanks!
comment:17 Changed 7 years ago by
 Description modified (diff)
comment:18 Changed 7 years ago by
 Description modified (diff)
comment:19 Changed 7 years ago by
The scalar product that was implemented is fundamental in Kac' book and for that alone this is an important patch.
I got one doctest failure in weight_space.py:
sage t weight_space.py ********************************************************************** File "weight_space.py", line 131, in sage.combinat.root_system.weight_space.WeightSpace Failed example: for ct in CartanType.samples(crystallographic=True)+[CartanType(["A",2],["C",5,1])]: TestSuite(ct.root_system().weight_lattice()).run() TestSuite(ct.root_system().weight_space()).run() Expected nothing Got: Failure in _test_not_implemented_methods: Traceback (most recent call last): [snip] AssertionError: Not implemented method: _symmetric_form_matrix
In cartan_type.py
you could delete the sentence on line 108 since you do implement Coxeter numbers!
For the positive roots, imaginary roots, etc. The user might like to know how to get a list of them. For *untwisted* types you can do this.
sage: PR=RootSystem(['A',3,1]).root_lattice().positive_real_roots() sage: [PR.unrank(i) for i in [0..4]] [alpha[1], alpha[2], alpha[3], alpha[1] + alpha[2], alpha[2] + alpha[3]]
Is there a more obvious way? And should something like this be in the doctest?
In the method basic_imaginary_roots the docstring still refers to simple imaginary roots (twice).
It would be good if there were pointers to Kac' book in some places. For example, in symmetric_form, the doc could have a pointer to Chapter 6. The Coxeter number method could have a pointer to Bourbaki, Lie Groups and Lie Algebras V.6.1.
comment:20 Changed 7 years ago by
I've created #16320 to make it easier to access elements in an infinite list (but I have not merged it in so it's not a dependency). I'm not sure if at the end of the day we want to keep using DisjointUnionEnumeratedSets
and possibly adding better general support for those or making our own special subclasses and having them be better targeted for the roots here. Nicolas, do you have any thoughts/comments on this?
IMO this can be left for a followup ticket unless there is a simple and clear better solution. Although if #16320 is a good way to go, I'll merge that in and update the doctests accordingly.
comment:21 Changed 7 years ago by
 Commit changed from 18fd0946874e2e54a75cdcd761311dea3f2810a4 to 760194b3d8ce7cb38574208255e65b4b8ef62a85
comment:22 Changed 7 years ago by
Still two doctest failures in root_lattice_realizations.py
.
sage t root_lattice_realizations.py ********************************************************************** File "root_lattice_realizations.py", line 634, in sage.combinat.root_system.root_lattice_realizations.RootLatticeRealizations.ParentMethods.positive_real_roots Failed example: PRR = L.positive_real_roots(); PRR Expected nothing Got: Positive real roots of type ['A', 3, 1] ********************************************************************** File "root_lattice_realizations.py", line 635, in sage.combinat.root_system.root_lattice_realizations.RootLatticeRealizations.ParentMethods.positive_real_roots Failed example: [PRR.unrank(i) for i in range(10)] Expected: Positive real roots of type ['A', 3, 1] [alpha[1], snip
comment:23 Changed 7 years ago by
 Commit changed from 760194b3d8ce7cb38574208255e65b4b8ef62a85 to eab1afea3f8f66c65802d2a7362928d65b47ea77
Branch pushed to git repo; I updated commit sha1. New commits:
eab1afe  Fixed doctest.

comment:24 Changed 7 years ago by
Whoops, forgot to commit.
comment:25 Changed 7 years ago by
All tests passed!
The following breaks:
[RootSystem?(['A',2,2]).root_lattice().positive_roots().unrank(x) for x in [0..10]]
It works for untwisted types. Is this a limitation of the patch or an unexpected failure?
If it is a limitation of the patch maybe it should be noted in the patch description. Also it might be better to fail with a not implemented error than with:
TypeError: unsupported operand type(s) for //: 'RootSpace_with_category.element_class' and 'int'
comment:26 Changed 7 years ago by
 Commit changed from eab1afea3f8f66c65802d2a7362928d65b47ea77 to 66f5674bf4281706f71f01202091ed02e1ee8bcc
Branch pushed to git repo; I updated commit sha1. New commits:
66f5674  Fixed type BC positive roots.

comment:27 Changed 7 years ago by
That was an error. Fixed now.
comment:28 Changed 7 years ago by
What about this?
sage: RootSystem(['D',4,2]).root_lattice().positive_roots()  AttributeError Traceback (most recent call last) <ipythoninput11ea364fc7aada> in <module>() > 1 RootSystem(['D',Integer(4),Integer(2)]).root_lattice().positive_roots() snip  AttributeError: 'NoneType' object has no attribute 'dual'
BTW Sage reports this root system as Root lattice of the Root system of type ['C', 3, 1]^{* }
If I have my facts straight Macdonald introduced a classification of affine root systems in his paper "Affine root systems and the Dedekind eta function" Inventiones 1972 but Kac didn't follow Macdonald. He introduced his own classification. So we are entering root systems in Kac' notation and they are getting converted automatically to Macdonald's notation. Of course that policy was not introduced by this patch but I mention that I've personally found it a little confusing.
comment:29 Changed 7 years ago by
 Commit changed from 66f5674bf4281706f71f01202091ed02e1ee8bcc to 7e8391003096463970d0ee0254d2043930d047d0
comment:30 Changed 7 years ago by
Another (stupid) error on my part. Fixed.
Also if you want to display in Kac's notation, you can do the following (it's somewhat hidden at the bottom of CatanType?
):
sage: CartanType.global_options(notation='Kac') sage: CartanType(['D',4,2]) ['D', 4, 2]
comment:31 Changed 7 years ago by
 Commit changed from 7e8391003096463970d0ee0254d2043930d047d0 to 1ffb4bb5a6cde5edc337357f463a6efa1c91d068
comment:32 Changed 7 years ago by
The following returns an error:
sage: sr=RootSystem(['A',6,2]).weight_lattice().simple_roots() sage: sr[0].symmetric_form(sr[1]) ... TypeError: no conversion of this rational to integer
There is no error if we use the root_lattice.
comment:33 Changed 7 years ago by
 Commit changed from 1ffb4bb5a6cde5edc337357f463a6efa1c91d068 to bccea25662638e48c34e91e3051f47a821613f90
Branch pushed to git repo; I updated commit sha1. New commits:
bccea25  Worked around a minor coercion issue and added doctests.

comment:34 Changed 7 years ago by
Hey Dan,
I've fixed the issue; the matrix stack()
command doesn't coerce (well, it's more of I've worked around it and is now #16399). I've added some more doctests about this. I'll merge in the latest beta later tonight.
comment:35 Changed 7 years ago by
 Commit changed from bccea25662638e48c34e91e3051f47a821613f90 to f4cee52a23d5d1c0092100981c46c04dd59dbc37
Branch pushed to git repo; I updated commit sha1. New commits:
f4cee52  Merge branch 'develop' into public/combinat/root_systems/improvements15384

comment:36 Changed 7 years ago by
 Reviewers changed from Dan Bump to Daniel Bump
 Status changed from needs_review to positive_review
This patch implements some important things from Chapter 6 of Kac' book for both twisted and untwisted affine Lie algebras, as methods of the root_lattice_realization or the weight_lattice_realization. These include a method of enumerating the positive roots (both real and imaginary), the invariant bilinear form, and a few other things. I tested with Kac' book in front of me, and all the problems I found have been addressed.
comment:37 Changed 7 years ago by
Thank you for doing the review Dan.
comment:39 Changed 7 years ago by
 Commit changed from f4cee52a23d5d1c0092100981c46c04dd59dbc37 to 72859401196f2a2893043c555146ce56102acabf
Branch pushed to git repo; I updated commit sha1. New commits:
7285940  Fixed bad merging in simple_roots_tilde.

comment:40 Changed 7 years ago by
 Status changed from needs_work to positive_review
Bad merging on my part; ended up overwriting simple_roots_tilde()
.
comment:41 Changed 7 years ago by
 Branch changed from public/combinat/root_systems/improvements15384 to 72859401196f2a2893043c555146ce56102acabf
 Resolution set to fixed
 Status changed from positive_review to closed
New commits: