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

Implementation of Littlewood-Richardson tableaux #21615

Closed
anneschilling opened this issue Sep 30, 2016 · 31 comments
Closed

Implementation of Littlewood-Richardson tableaux #21615

anneschilling opened this issue Sep 30, 2016 · 31 comments

Comments

@anneschilling
Copy link

This patch implements a new class for Littlewood-Richardson tableaux.

CC: @MariaMonks @sagetrac-j-levinson @tscrim

Component: combinatorics

Author: Maria Gillespie, Anne Schilling, Jake Levinson

Branch/Commit: 40414a3

Reviewer: Travis Scrimshaw

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

@anneschilling
Copy link
Author

comment:2

This is a first implementation of LR tableaux. Things to improve:

  • allow for skew LR tableaux

  • improve the iterator (it is currently a dumb iterator selecting semistandard tableaux that are LR with respect to a given sequence of weights)


New commits:

ec8d39cfirst implementation of Littlewood-Richardson tableaux

@anneschilling
Copy link
Author

Commit: ec8d39c

@anneschilling
Copy link
Author

Author: Maria Gillespie, Anne Schilling

@anneschilling

This comment has been minimized.

@anneschilling
Copy link
Author

Branch: u/aschilling/LR-tableaux-21615

@tscrim
Copy link
Collaborator

tscrim commented Oct 6, 2016

comment:3

Hey Maria and Anne,

I've taken a look over this and there are a few polishing things that need to be done.

  • You should have documentation for each of the classes. This is important both for locality of documentation and so LittlewoodRichardsonTableau? does not show the __init__ documentation. You can just put the INPUT: block at the class level doc as well.

  • For error messages, they are typically not considered complete sentences, and should not start with a capital letter or end with punctuation.

  • In the documentation, double backticks is code formatting, single is latex. So you should change, e.g., `self` to ``self``.

  • For inputs, we are trying to standardize them to the form

    - ``arg`` -- (optional) some explanation
    

    (note, also no punctuation).

It also seems like what you want LR tableaux to inherit from is SemistandardTableaux_shape_weight. This way you can just use the base class iterator instead of creating a new parent. Actually, better yet, you could just call for t in symmetrica.kostka_tab(self.shape, self.weight):. As of right now, your iterator does not create instances of LittlewoodRichardsonTableau:

sage: from sage.combinat.lr_tableau import LittlewoodRichardsonTableaux
sage: LR = LittlewoodRichardsonTableaux([3,2,1],[[2,1],[2,1]])
sage: LR[0]
[[1, 1, 3], [2, 3], [4]]
sage: type(_)
<class 'sage.combinat.tableau.SemistandardTableaux_shape_weight_with_category.element_class'>

@tscrim
Copy link
Collaborator

tscrim commented Oct 6, 2016

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2016

Changed commit from ec8d39c to d126923

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2016

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

d126923fixed most of Travis' comments

@anneschilling
Copy link
Author

comment:5

Hi Travis,

Thanks for your comments. I fixed most of the issues you mention.

It also seems like what you want LR tableaux to inherit from is SemistandardTableaux_shape_weight. This way you can just use the base class iterator instead of creating a new parent. Actually, better yet, you could just call for t in symmetrica.kostka_tab(self.shape, self.weight):.

This suggestion does not seem to work. We are anyway planning to implement a smarter iterator, so it is probably not worth it to call symmetrica.

Anne

@tscrim
Copy link
Collaborator

tscrim commented Oct 15, 2016

@tscrim
Copy link
Collaborator

tscrim commented Oct 15, 2016

Changed commit from d126923 to 3490837

@tscrim
Copy link
Collaborator

tscrim commented Oct 15, 2016

comment:6

I reworked things around a bit so that the iterator doesn't create intermediate instances of SemistandardTableau. I also added a __contains__ method and subsequently reworked the check to avoid duplication. I also did a few other doc and little things, including lazily importing the LR tableaux into the global namespace. Let me know if you want to do the iterator on this ticket or on a followup.


New commits:

7e45dc9Merge branch 'u/aschilling/LR-tableaux-21615' of git://trac.sagemath.org/sage into public/combinat/RC_tableaux-21615
3490837Some reviewer changes and fixes.

@anneschilling
Copy link
Author

comment:7

There seems to be a doctest failure now:

sage -t finite_word.py
**********************************************************************
File "finite_word.py", line 606, in sage.combinat.words.finite_word.FiniteWord_class.is_yamanouchi
Failed example:
    w.is_yamanouchi(n=3)
Exception raised:
    Traceback (most recent call last):
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.words.finite_word.FiniteWord_class.is_yamanouchi[5]>", line 1, in <module>
        w.is_yamanouchi(n=Integer(3))
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/finite_word.py", line 619, in is_yamanouchi
        w = Word(self, alphabet = range(1,n+1))
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/word.py", line 208, in Word
        parent = Words(alphabet)
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/words.py", line 101, in Words
        return FiniteOrInfiniteWords(alphabet)
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/words.py", line 1722, in __init__
        AbstractLanguage.__init__(self, alphabet)
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/words.py", line 149, in __init__
        alphabet = build_alphabet(alphabet)
      File "/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/words/alphabet.py", line 268, in build_alphabet
        raise ValueError("unable to construct an alphabet from the given parameters")
    ValueError: unable to construct an alphabet from the given parameters
**********************************************************************
1 item had failures:
   1 of  12 in sage.combinat.words.finite_word.FiniteWord_class.is_yamanouchi
    [1270 tests, 1 failure, 3.31 s]

I do not recall that it was there before, but I could be mistaken!

@tscrim
Copy link
Collaborator

tscrim commented Oct 17, 2016

comment:8

This comes from the fact that I used a new beta of sage, where words are now using Python3 style range. It is not a list anymore, so the simplest fix is to wrap it in a list.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2016

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

ecf10efSome changes to finite word, including Python3 compatibility.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2016

Changed commit from 3490837 to ecf10ef

@anneschilling
Copy link
Author

comment:10

I think this can go in now. We can reimplement the iterator later!

@tscrim
Copy link
Collaborator

tscrim commented Oct 20, 2016

comment:11

Then if you're happy with my latest changes, then you can set a positive review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2016

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

9c67f19added an implementation of LR tableaux using Buch's lrcalc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2016

Changed commit from ecf10ef to 9c67f19

@sagetrac-j-levinson
Copy link
Mannequin

sagetrac-j-levinson mannequin commented Oct 24, 2016

comment:14

Hi all,

I wrote an iterator, LRtabs_multi, that calls Anders Buch's fast lrcalc code (which can compute LR tableaux with specified skew shape). The tableaux we want can be found using a sequence of lrcalc calls.

I didn't rewrite the current __iter__ method because this new one is slightly more general (it can compute skew tableaux rather than straight shape tableaux).

-Jake

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2016

Changed commit from 9c67f19 to 1d3fe7c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2016

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

1a8576aMerge branch 'develop' into public/combinat/LR_tableaux-21615
1a222e9Merge branch 'public/combinat/LR_tableaux-21615' of git://trac.sagemath.org/sage into public/combinat/LR_tableaux-21615
90dc8e3Merge branch 'public/combinat/LR_tableaux-21615' of git://trac.sagemath.org/sage into public/combinat/LR_tableaux-21615
8c63ec6Merge branch 'develop' into public/combinat/LR_tableaux-21615
a5666banew iterator along Jake's ideas
1d3fe7cadded Jake's name to authors

@anneschilling
Copy link
Author

comment:16

Hi Jake and all,

Thank you for your improvements. I rewrote the __iter__ method that uses lrskew and your method _tableauJoin (which I renamed _tableau_join). It seems shorter than what you had as it does not need standardization or destandardization. See what you think. It should be relatively easy to upgrade the whole class to allow for SkewTableaux as well. But perhaps we can leave that for another
ticket?

Best,

Anne

@dimpase dimpase modified the milestones: sage-7.4, sage-7.5 Oct 26, 2016
@vbraun vbraun modified the milestones: sage-7.5, sage-7.6 Oct 29, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 30, 2016

Changed commit from 1d3fe7c to 40414a3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 30, 2016

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

40414a3Some last little tweaks.

@tscrim
Copy link
Collaborator

tscrim commented Oct 30, 2016

Changed author from Maria Gillespie, Anne Schilling to Maria Gillespie, Anne Schilling, Jake Levinson

@tscrim
Copy link
Collaborator

tscrim commented Oct 30, 2016

comment:20

I made a few little more tweaks. If my changes look good, you can set it back to a positive review.

@tscrim tscrim modified the milestones: sage-7.6, sage-7.5 Oct 30, 2016
@anneschilling
Copy link
Author

comment:21

Thanks for the review!

@vbraun
Copy link
Member

vbraun commented Nov 7, 2016

Changed branch from public/combinat/LR_tableaux-21615 to 40414a3

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

4 participants