Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Folded Cartan types #13871

Closed
tscrim opened this issue Dec 28, 2012 · 39 comments
Closed

Folded Cartan types #13871

tscrim opened this issue Dec 28, 2012 · 39 comments

Comments

@tscrim
Copy link
Collaborator

tscrim commented Dec 28, 2012

This implements a new class CartanTypeFolded which takes a Cartan type and considers the diagram folding and stores the associated data.

This also cleans up the references in kirillov_reshetikhin.py.


Apply: attachment: trac_13871-virtual_cartan_type-ts.patch

CC: @sagetrac-sage-combinat @anneschilling @nthiery

Component: combinatorics

Keywords: Cartan, root systems

Author: Travis Scrimshaw

Reviewer: Frédéric Chapoton, Anne Schilling, Nicolas M. Thiéry

Merged: sage-5.12.beta5

Issue created by migration from https://trac.sagemath.org/ticket/13871

@tscrim tscrim added this to the sage-5.11 milestone Dec 28, 2012
@tscrim tscrim self-assigned this Dec 28, 2012
@tscrim
Copy link
Collaborator Author

tscrim commented Feb 14, 2013

Fixed doctests

@tscrim
Copy link
Collaborator Author

tscrim commented Feb 14, 2013

comment:3

Attachment: trac_13838-virtual_kleber_alg-ts.patch.gz

Uploaded wrong file...

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 11, 2013

comment:4

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@fchapoton
Copy link
Contributor

comment:5

found a typo here :

there exists am affine

@fchapoton

This comment has been minimized.

@tscrim
Copy link
Collaborator Author

tscrim commented Apr 19, 2013

comment:6

Replying to @fchapoton:

found a typo here :

there exists am affine

Thanks for catching that Frederic.

@fchapoton
Copy link
Contributor

comment:7

one doctest is failing (concerning latex representation)

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 9, 2013

comment:8

New version of the patch which does the changes that Nicolas and I discussed. Ready for another look.

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 9, 2013

comment:9

Explicitly cast the elements of scaling_factors() to python ints to simplify #13838 and #13872.

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@tscrim
Copy link
Collaborator Author

tscrim commented Jul 12, 2013

comment:10

Fixed typo in doctest.

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@fchapoton
Copy link
Contributor

Attachment: trac_13871_review.patch.gz

@fchapoton
Copy link
Contributor

comment:12

here is a review patch, just cleaning up and putting things into pep8 shape.

Once the bot has turned green again, and if you agree with my review patch, I will give a positive review, but there is a point that bothers me :

I have never heard the words "virtual cartan type" before. Could you give a reference to an article where this terminology is used ?

@anneschilling
Copy link
Contributor

comment:13

Replying to @fchapoton:

here is a review patch, just cleaning up and putting things into pep8 shape.

Once the bot has turned green again, and if you agree with my review patch, I will give a positive review, but there is a point that bothers me :

I have never heard the words "virtual cartan type" before. Could you give a reference to an article where this terminology is used ?

Nicolas and I are going to look at the patch this week since we had some specific comments about the implementation. So please wait until we have looked at it!

Thank you,

Anne

@nthiery
Copy link
Contributor

nthiery commented Aug 21, 2013

comment:14

Hi Travis!

We finally had a brainstorm with Anne about this ticket. Sorry that
they come so late in the process, but we have some suggestions on how
to improve the user interface and so.

  • Documentation improvements for the main class:

    Start from the definition of folding of a simply laced Dynkin
    diagram from :wikipedia:Dynkin_diagram (note that, unlike the
    current documentation suggests it's not specific to affine).

    Say that the object in Sage models the Cartan type X, endowed
    with the folding from \hat(X).

    Mention that, in the affine case, the code makes the assumption
    that the special node is fixed.

    Mention applications to crystals, with refs.

    For the scaling factors, start from a conceptual type free
    definition. Maybe, following [Schilling FPSAC 2006 p.4] something
    like: the \gamma_i are (up to overall factor?) the unique numbers
    so that:

      \Lambda_i   ->    \gamma_i \sum_j \Lambda_{j\in\orbit(i)}
    

    is a proper embedding from the weight lattice of X to that of
    \hat{X}?

    Then give the combinatorial description in term of the Dynkin
    diagram and the type-free formula in affine type.

  • Suggestions for the user interface:

    sage: ct = CartanType(["G",2]); ct
    ['G',2]
    sage: vct = ct.as_folding(); vct
    ['G', 2] as folding of ['D', 4]
    sage: type(vct)
    <class '....type_folded.CartanType'>
    sage: vct.folding_of()                 # replaces virtual
    ['D', 4]
    sage: sigma = vct.folding_automorphism()       # replaces sigma
    
  • Put the implementation accordingly in .../root_system/type_folded.py

  • Does scaling_factors work for type BC relabelled?

  • Does the type free formula for the scaling factors work for type
    BC/dual? If not, the doc needs to be updated.

Thanks for this useful addition!

Cheers,
Anne and Nicolas

@anneschilling
Copy link
Contributor

comment:15

Hi Travis,

We forgot a few more things:

  • You seem to only work with the orbit of the Dynkin diagram automorphism. Hence the name
    sage: sigma = vct.folding_orbit()       # replaces sigma

might actually be more appropriate.

  • The output of the orbit is a dictionary. Hence it would make more sense to also take as an input a dictionary with keys the Dynkin labels. In a list it is not completely clear which label go to which (since for example in the finite case the Dynkin nodes are labeled from 1 to n rather than 0 to n). The output could actually be a family instead of a dictionary.

Thanks for your work on this!

Nicolas and Anne

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 22, 2013

comment:16

Here's the new version of the patch with your suggested changes. Thanks all for reviewing this.

Best,

Travis

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch​

@anneschilling
Copy link
Contributor

comment:17

Hi Travis,

Thanks for the update. Here are a couple more comments:

  • Do you really want to keep sigma as a variable or rather change it to orbit since this is what the input is? Also, when you describe what the INPUT is, please be more specific that you are inputting the orbit of a Dynkin diagram automorphism and not the automorphism itself. This is currently confusing in the documentation. Specify that this should be inputted as a dictionary.

  • Please add explanations that this is used for crystals (which results into virtual crystals) and give the appropriate references.

Thanks!

Anne

@anneschilling
Copy link
Contributor

comment:19

Hi Travis,

Thank you for making the changes. Could you please also add in INPUT that the orbit is a dictionary to be clear!?

Did you investigate the questions regarding relabelings that we posed?

I get doctest failures

sage -t pieri_factors.py
**********************************************************************
File "pieri_factors.py", line 352, in sage.combinat.root_system.pieri_factors.PieriFactors_affine_type.maximal_elements
Failed example:
    [w.reduced_word() for w in PF.maximal_elements()]
Expected:
    [[0, 2, 3, 2, 0], [1, 0, 2, 3, 2], [2, 3, 2, 1, 0], [3, 2, 1, 0, 2], [1, 2, 3, 2, 1], [2, 1, 0, 2, 3]]
Got:
    [[0, 2, 3, 2, 0], [1, 0, 2, 3, 2], [2, 3, 2, 1, 0], [1, 2, 3, 2, 1], [3, 2, 1, 0, 2], [2, 1, 0, 2, 3]]
**********************************************************************
File "pieri_factors.py", line 987, in sage.combinat.root_system.pieri_factors.PieriFactors_type_D_affine
Failed example:
    [u.reduced_word() for u in PF.maximal_elements()]
Expected:
    [[0, 2, 3, 5, 4, 3, 2, 0], [1, 0, 2, 3, 5, 4, 3, 2], [2, 3, 5, 4, 3, 2, 1, 0], [2, 1, 0, 2, 3, 5, 4, 3], [1, 2, 3, 5, 4, 3, 2, 1], [3, 5, 4, 3, 2, 1, 0, 2], [3, 2, 1, 0, 2, 3, 5, 4], [5, 4, 3, 2, 1, 0, 2, 3], [5, 3, 2, 1, 0, 2, 3, 5], [4, 3, 2, 1, 0, 2, 3, 4]]
Got:
    [[0, 2, 3, 5, 4, 3, 2, 0], [1, 0, 2, 3, 5, 4, 3, 2], [2, 3, 5, 4, 3, 2, 1, 0], [3, 5, 4, 3, 2, 1, 0, 2], [1, 2, 3, 5, 4, 3, 2, 1], [2, 1, 0, 2, 3, 5, 4, 3], [5, 4, 3, 2, 1, 0, 2, 3], [3, 2, 1, 0, 2, 3, 5, 4], [5, 3, 2, 1, 0, 2, 3, 5], [4, 3, 2, 1, 0, 2, 3, 4]]
**********************************************************************

though I am not sure this is caused by this patch.

After compiling the documentation the link to [FOS09] does not seem to work.

Best,

Anne

@anneschilling
Copy link
Contributor

Reviewer: Federic Chapoton, Anne Schilling, Nicolas Thiery

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 23, 2013

comment:21

Hey Anne,

I expanded on the doc for the INPUT: block. The Pieri factors errors occur both with and without my patch applied, so it doesn't seem to be this patch. The link to [FOS09] in both kirillov_reshetikhin.py and type_folded.py work for me. For the type BC scaling factors, I added a doctest showing it works under relabelling.

Best,

Travis

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 23, 2013

comment:22

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@anneschilling
Copy link
Contributor

comment:23

Hi Travis,

Ok, looks good now. I let Frederic finish the technical review that he started!

Best,

Anne

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 23, 2013

comment:24

Here's the patch with Frederic's review patch changes as well. I forgot to fold in the review patch before making Anne and Nicolas' changes.

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@fchapoton
Copy link
Contributor

comment:25

apply only trac_13871-virtual_cartan_type-ts.patch

@fchapoton
Copy link
Contributor

Changed reviewer from Federic Chapoton, Anne Schilling, Nicolas Thiery to Frédéric Chapoton, Anne Schilling, Nicolas Thiery

@nthiery
Copy link
Contributor

nthiery commented Aug 24, 2013

Changed reviewer from Frédéric Chapoton, Anne Schilling, Nicolas Thiery to Frédéric Chapoton, Anne Schilling, Nicolas M. Thiéry

@fchapoton
Copy link
Contributor

Work Issues: doctest

@fchapoton
Copy link
Contributor

comment:28

there is a failing doctest that needs to be corrected

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 25, 2013

comment:29

Here's with the fixed doctest in type_relabel.py. IDK about the one in weierstrass.py, seems completely unrelated to this patch, and it passed for me:

sage -t --long ../../schemes/toric/weierstrass.py
    [134 tests, 7.30 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 7.5 seconds
    cpu time: 6.0 seconds
    cumulative wall time: 7.3 seconds

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 25, 2013

Changed work issues from doctest to none

@fchapoton
Copy link
Contributor

comment:30

the role for arxiv is :arxiv: and not :arXiv:

otherwise, everything looks good to me

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 25, 2013

Fixed doctests

@tscrim
Copy link
Collaborator Author

tscrim commented Aug 25, 2013

comment:31

Attachment: trac_13871-virtual_cartan_type-ts.patch.gz

Fixed.

For patchbot:

Apply: trac_13871-virtual_cartan_type-ts.patch

@fchapoton
Copy link
Contributor

comment:32

ok, positive review

could you please update the description of the ticket ?

@tscrim

This comment has been minimized.

@tscrim tscrim changed the title Virtual Cartan types Folded Cartan types Aug 29, 2013
@tscrim
Copy link
Collaborator Author

tscrim commented Aug 29, 2013

comment:34

Thank you all for doing the review.

@jdemeyer
Copy link
Contributor

jdemeyer commented Sep 4, 2013

Merged: sage-5.12.beta5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants