Opened 7 years ago

Closed 7 years ago

#15384 closed enhancement (fixed)

Improvements to root systems

Reported by: tscrim Owned by: sage-combinat
Priority: major Milestone: sage-6.3
Component: combinatorics Keywords: root systems, days54
Cc: sage-combinat, 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:

Status badges

Description (last modified by tscrim)

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 A2n(2) would give A2n).
  • (Dual) Coxeter numbers for finite types.

Change History (41)

comment:1 Changed 7 years ago by tscrim

  • Branch set to public/combinat/root_systems/improvements-15384
  • Commit set to 0fa598a6ae59bc8a336e93140998a8db0e26a081

New commits:

0fa598aimported patch root_system_data-ts.patch
870837fimported patch root_systems-ts.patch

comment:2 Changed 7 years ago by tscrim

  • Keywords days54 added

comment:3 Changed 7 years ago by git

  • Commit changed from 0fa598a6ae59bc8a336e93140998a8db0e26a081 to e40f89241889ecdc2d7f5fc8ad2d623f75584bc3

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

e40f892Changed horizontal to basic_untwisted.
930782bFixed indentation error.
f87789dMerge branch 'master' into public/combinat/root_systems/improvements-15384

comment:4 Changed 7 years ago by git

  • Commit changed from e40f89241889ecdc2d7f5fc8ad2d623f75584bc3 to b4c7865c9dff6241915b6491c5f097486ee052df

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

b4c7865Merge branch 'master' into public/combinat/root_systems/improvements-15384

comment:5 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:6 Changed 7 years ago by git

  • Commit changed from b4c7865c9dff6241915b6491c5f097486ee052df to dd976c77935b077ef7917c413e1357b9254801f5

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

6100a4bMerge branch 'public/combinat/root_systems/improvements-15384' of trac.sagemath.org:sage into public/combinat/root_systems/improvements-15384
a5a6b1cMerge branch 'develop' into public/combinat/root_systems/improvements-15384
f802ae1Merge branch 'develop' into public/combinat/root_systems/improvements-15384
d83982cMerge branch 'develop' into public/combinat/root_systems/improvements-15384
ecf8f76Merge branch 'develop' into public/combinat/root_systems/improvements-15384
c52545cMerge branch 'develop' into public/combinat/root_systems/improvements-15384
14c367eMerge branch 'develop' into public/combinat/root_systems/improvements-15384
fe68bf6Merge branch 'develop' into public/combinat/root_systems/improvements-15384
896e682Merge branch 'develop' into public/combinat/root_systems/improvements-15384
dd976c7Merge branch 'develop' into public/combinat/root_systems/improvements-15384

comment:7 Changed 7 years ago by git

  • Commit changed from dd976c77935b077ef7917c413e1357b9254801f5 to 2c7f793553819baad0d054082c69e029edd42e98

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

2c7f793Merge branch 'develop' into public/combinat/root_systems/improvements-15384

comment:8 Changed 7 years ago by git

  • Commit changed from 2c7f793553819baad0d054082c69e029edd42e98 to 122ca6eca844a35e402b30cbbf71efcc6afb1e2a

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

ea1641fMerge branch 'develop' into public/combinat/root_systems/improvements-15384
054b37cMerge branch 'develop' into public/combinat/root_systems/improvements-15384
30de9e8Merge branch 'develop' into public/combinat/root_systems/improvements-15384
122ca6eMerge branch 'develop' into public/combinat/root_systems/improvements-15384

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 7 years ago by git

  • Commit changed from 122ca6eca844a35e402b30cbbf71efcc6afb1e2a to 64ac7a57e4b41d533e270b875d883cd7d910dae3

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

87cf9edMerge branch 'public/combinat/root_systems/improvements-15384' of trac.sagemath.org:sage into public/combinat/root_systems/improvements-15384
c5f9747Added placeholder for symmetric_form in WLR.
64ac7a5Fixed doctests.

comment:11 Changed 7 years ago by bump

  • Reviewers set to bump

comment:12 Changed 7 years ago by git

  • Commit changed from 64ac7a57e4b41d533e270b875d883cd7d910dae3 to 1817877be05246ac3248a6b8fc82a159dc27351c

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

1817877Almost there.

comment:13 Changed 7 years ago by git

  • Commit changed from 1817877be05246ac3248a6b8fc82a159dc27351c to 18fd0946874e2e54a75cdcd761311dea3f2810a4

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

18fd094Fixed affine weight lattice symmetric form.

comment:14 Changed 7 years ago by tscrim

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

Glad you got it fixed. I'll start looking at the patch.

comment:16 Changed 7 years ago by nthiery

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 tscrim

  • Description modified (diff)

comment:18 Changed 7 years ago by tscrim

  • Description modified (diff)

comment:19 Changed 7 years ago by bump

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.

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

comment:20 Changed 7 years ago by tscrim

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 follow-up 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 git

  • Commit changed from 18fd0946874e2e54a75cdcd761311dea3f2810a4 to 760194b3d8ce7cb38574208255e65b4b8ef62a85

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

cb88df7Merge branch 'develop' into public/combinat/root_systems/improvements-15384
760194bMore changes and fixes.

comment:22 Changed 7 years ago by bump

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 git

  • Commit changed from 760194b3d8ce7cb38574208255e65b4b8ef62a85 to eab1afea3f8f66c65802d2a7362928d65b47ea77

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

eab1afeFixed doctest.

comment:24 Changed 7 years ago by tscrim

Whoops, forgot to commit.

comment:25 Changed 7 years ago by bump

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 git

  • Commit changed from eab1afea3f8f66c65802d2a7362928d65b47ea77 to 66f5674bf4281706f71f01202091ed02e1ee8bcc

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

66f5674Fixed type BC positive roots.

comment:27 Changed 7 years ago by tscrim

That was an error. Fixed now.

comment:28 Changed 7 years ago by bump

What about this?

sage: RootSystem(['D',4,2]).root_lattice().positive_roots()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-11-ea364fc7aada> 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 git

  • Commit changed from 66f5674bf4281706f71f01202091ed02e1ee8bcc to 7e8391003096463970d0ee0254d2043930d047d0

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

e17ce53Merge branch 'develop' into public/combinat/root_systems/improvements-15384
7e83910Fixes for other twisted root systems.

comment:30 Changed 7 years ago by tscrim

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 git

  • Commit changed from 7e8391003096463970d0ee0254d2043930d047d0 to 1ffb4bb5a6cde5edc337357f463a6efa1c91d068

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

c91754dMerge branch 'develop' into public/combinat/root_systems/improvements-15384
82c6257Fixed doctest.
1ffb4bbFixed doctest and for the weight lattice.

comment:32 Changed 7 years ago by bump

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 git

  • Commit changed from 1ffb4bb5a6cde5edc337357f463a6efa1c91d068 to bccea25662638e48c34e91e3051f47a821613f90

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

bccea25Worked around a minor coercion issue and added doctests.

comment:34 Changed 7 years ago by tscrim

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 git

  • Commit changed from bccea25662638e48c34e91e3051f47a821613f90 to f4cee52a23d5d1c0092100981c46c04dd59dbc37

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

f4cee52Merge branch 'develop' into public/combinat/root_systems/improvements-15384

comment:36 Changed 7 years ago by bump

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

Thank you for doing the review Dan.

comment:38 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work

Doctests fail

comment:39 Changed 7 years ago by git

  • Commit changed from f4cee52a23d5d1c0092100981c46c04dd59dbc37 to 72859401196f2a2893043c555146ce56102acabf

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

7285940Fixed bad merging in simple_roots_tilde.

comment:40 Changed 7 years ago by tscrim

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

  • Branch changed from public/combinat/root_systems/improvements-15384 to 72859401196f2a2893043c555146ce56102acabf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.