Opened 2 years ago
Closed 2 years ago
#30750 closed enhancement (fixed)
Implementation of semistandard tableaux as pathtableaux
Reported by:  Bruce Westbury  Owned by:  Bruce Westbury 

Priority:  major  Milestone:  sage9.3 
Component:  combinatorics  Keywords:  cactus group, BenderKnuth 
Cc:  Kevin Dilks, Martin Rubey, Stephan Pfannerer, Darij Grinberg  Merged in:  
Authors:  Bruce Westbury  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  13885d1 (Commits, GitHub, GitLab)  Commit:  13885d110f96644d6d299253fe48f4a91442b4bc 
Dependencies:  #30870  Stopgaps: 
Description
This is an implementation of the abstract base class PathTableau?. This implementation is for semistandard tableaux, represented as a chain of partitions (essentially, the GelfandTsetlin pattern). This generalises the jeudetaquin operations of rectification, promotion, evacuation from standard tableaux to semistandard tableaux. The local rule is the BenderKnuth involution.
Change History (35)
comment:1 Changed 2 years ago by
Keywords:  cactus group BenderKnuth added 

Owner:  set to Bruce Westbury 
Type:  PLEASE CHANGE → enhancement 
comment:2 Changed 2 years ago by
Branch:  → u/Bruce/semistandardpath 

comment:3 Changed 2 years ago by
Commit:  → eb2c9c97dc714563bb6304fc4773b0ccb410ba6f 

comment:4 Changed 2 years ago by
Status:  new → needs_review 

comment:5 Changed 2 years ago by
Cc:  Kevin Dilks Martin Rubey Stephan Pfannerer Darij Grinberg added 

comment:6 Changed 2 years ago by
Commit:  eb2c9c97dc714563bb6304fc4773b0ccb410ba6f → a90f69e234ad3bb308ec5eede6c925ffdc3421ea 

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

comment:7 Changed 2 years ago by
Commit:  a90f69e234ad3bb308ec5eede6c925ffdc3421ea → b83bd7d7b0b2a86dd92e7af60c4d4696103ce300 

Branch pushed to git repo; I updated commit sha1. New commits:
b83bd7d  Removed trailing white spaces

comment:8 Changed 2 years ago by
Commit:  b83bd7d7b0b2a86dd92e7af60c4d4696103ce300 → 48fa41ff2a33a688cd5385cb1d1147a27a11b6f3 

Branch pushed to git repo; I updated commit sha1. New commits:
48fa41f  Merge branch 'develop' into semistandardpath

comment:9 Changed 2 years ago by
First lines of docs should say "Convert", "Check", etc instead of "Converts", "Checks", etc
comment:10 Changed 2 years ago by
Commit:  48fa41ff2a33a688cd5385cb1d1147a27a11b6f3 → 89ba5acb76b11120b45d141090867d18ae8cf81a 

comment:11 Changed 2 years ago by
Commit:  89ba5acb76b11120b45d141090867d18ae8cf81a → 57cf0fbc8a7cc5f8b9fbeb32d2bb23ed2a3bb058 

Branch pushed to git repo; I updated commit sha1. New commits:
57cf0fb  Added verbose option

comment:12 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:14 Changed 2 years ago by
Because the sage installation of the patchbot has a failing doctest. Nothing to do with your ticket. So far, nobody has taken up the task to fix this issue with pexpect and singular interface.
comment:15 Changed 2 years ago by
missing a double colon here
+ TESTS:
The following should not be indented (never indent after a single colon)
+ INPUT: + + * a sequence of partitions + * a sequence of lists/tuples + * a semistandard tableau + * a GelfandTsetlin pattern
comment:16 Changed 2 years ago by
This should take double backticks
+ Return `True` if `self` is skew.
Single backticks are for latex code (same as $)
comment:18 Changed 2 years ago by
Commit:  57cf0fbc8a7cc5f8b9fbeb32d2bb23ed2a3bb058 → f72ef26b8a26d6a8e88dc70aadf19a43d0d79970 

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

comment:19 Changed 2 years ago by
Commit:  f72ef26b8a26d6a8e88dc70aadf19a43d0d79970 → fc66fcae93bdadfdb5d3b08533d59e26a73de8c2 

Branch pushed to git repo; I updated commit sha1. New commits:
fc66fca  More corrections to doc strings

comment:20 Changed 2 years ago by
Thanks. Now the syntax of
+ The :class:`SemistandardSkewTableau is not implemented.
is missing the backtick after SemistandardSkewTableau
comment:21 Changed 2 years ago by
Commit:  fc66fcae93bdadfdb5d3b08533d59e26a73de8c2 → 4ca70e1840829f4ddb38d98a0bac3f40dbc95d10 

Branch pushed to git repo; I updated commit sha1. New commits:
4ca70e1  A correction

comment:22 Changed 2 years ago by
A few minor comments:
I would rename SemistandardPath
to be SemistandardPathTableau
to make the name more explicit. Also SemistandardPaths
is grammatically wrong as you want SemistandardPathTableaux
.
You should add more explanation there for the INPUT:
, mainly stating it "Can be one of the following:". There also needs to be at least one test for each of these input types. Can it also take a skew semistandard tableau? I think it should be able to. How about a skew partition then regular partitions? (This one, I would say it is fine the leave off and if someone does give this, I would be a bit surprised.)
For check
, it would be good to have actual EXAMPLES::
.
local_rule
should have a 1line description, then the rest of it should be separated in another paragraph with more details.
Error messages should start with lowercase letters (this is a Python convention we try to follow).
In rectify()
, instead of using Partition(data)
, it would be faster to use _Partitions(data)
. Although it would be best to not actually construct partitions, but instead keep them as tuples. Similarly, instead of calling SemistandardPath
, you can do P.element_class(P, data)
, where P = self.parent()
.
if inner == None: +if inner is None:
return self[0] != () +return bool(self[0])
 lt = list(self)  if lt[0] == ():  lt = lt[1:]  lt.reverse() + if not lt[1]: + lt.pop() return GelfandTsetlinPattern([list(a) for a in lt])
In _test_jdt_promotion
, could you make the tryexcept
block more local to the line you expect to fail? It looks weird to have it around the actual test run.
General PEP8 stuff such as spaces
def rectify(self,inner=None,verbose=False): +def rectify(self, inner=None, verbose=False):
 for i in range(1,len(self)1):  if not all(r>=s for r,s in zip(self[i+1],self[i])): + for i in range(1, len(self)1): + if not all(r >= s for r, s in zip(self[i+1], self[i])):
(but not necessarily limited to these).
comment:23 Changed 2 years ago by
Authors:  Bruce → Bruce Westbury 

comment:24 Changed 2 years ago by
Commit:  4ca70e1840829f4ddb38d98a0bac3f40dbc95d10 → 82ce2e7689dddc627396c9d76b3134802a559dda 

comment:25 Changed 2 years ago by
Commit:  82ce2e7689dddc627396c9d76b3134802a559dda → cf2be3b9a6bc7768b69e79e8bcfe957c23d2ec26 

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

comment:26 Changed 2 years ago by
Reviewers:  tscrim → Travis Scrimshaw 

Thank you. A few other little things I noticed on a second pass:
 You need to add the file to the big combinat module list for the documentation. (Actually, we forgot this for the frieze patterns. You can add it in here too if you want, but perhaps another ticket.)
 I think this is better formatting since it is code input:
This is implemented by toggling the entries of the `i`th list. The allowed range for `i` is 0 < `i` < len(self)1 so any row except +This is implemented by toggling the entries of the ``i``th list. +The allowed range for ``i`` is ``0 < i < len(self)1`` so any row except
if lt[1] == ():
>if not lt[1]:
comment:27 Changed 2 years ago by
Commit:  cf2be3b9a6bc7768b69e79e8bcfe957c23d2ec26 → aa7526ff3a265e8d188a6a77e67cd1338e31fac2 

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

comment:28 Changed 2 years ago by
Commit:  aa7526ff3a265e8d188a6a77e67cd1338e31fac2 → 6b69171305945e3c3363b87ceea8325e1d5db7be 

Branch pushed to git repo; I updated commit sha1. New commits:
6b69171  Added module to documentation

comment:29 Changed 2 years ago by
The doctests in is_integral
are not indented, which is causing the docbuild to fail.
comment:30 Changed 2 years ago by
This will also likely conflict with #30870 and should have that as a dependency.
comment:31 Changed 2 years ago by
Commit:  6b69171305945e3c3363b87ceea8325e1d5db7be → 414ea304a4f6e1e635f01b014ff68feac3cdc5bd 

Branch pushed to git repo; I updated commit sha1. New commits:
414ea30  Indented Examples in is_integral

comment:32 Changed 2 years ago by
Dependencies:  → #30870 

comment:33 Changed 2 years ago by
Commit:  414ea304a4f6e1e635f01b014ff68feac3cdc5bd → 13885d110f96644d6d299253fe48f4a91442b4bc 

comment:34 Changed 2 years ago by
Status:  needs_review → positive_review 

comment:35 Changed 2 years ago by
Branch:  u/Bruce/semistandardpath → 13885d110f96644d6d299253fe48f4a91442b4bc 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Started testing
More testing
More testing
More testing, TestSuite fails
Took out Partitions
Passed unit tests
All tests passed.