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 weak k-tableaux #12250

Closed
anneschilling opened this issue Jan 2, 2012 · 25 comments
Closed

Implementation of weak k-tableaux #12250

anneschilling opened this issue Jan 2, 2012 · 25 comments

Comments

@anneschilling
Copy link

This patch implements a new class of weak k-tableaux. It also removes white spaces from core.py.

The methods on k-charge were contributed by Nate Gallup and Avi Dalal during Sage Days 49 in Orsay.

Depends on #14519
Depends on #14101
Depends on #7983
Depends on #14772
Depends on #13589
Depends on #14907
Depends on #10630
Depends on #14143
Depends on #14015
Depends on #14516

CC: @sagetrac-sage-combinat @sagetrac-npgallup

Component: combinatorics

Keywords: tableaux, days49

Author: Anne Schilling

Reviewer: Mike Zabrocki, Travis Scrimshaw

Merged: sage-5.12.beta5

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

@anneschilling
Copy link
Author

Changed keywords from tableaux to tableaux, days49

@anneschilling
Copy link
Author

Changed dependencies from #11742 to none

@anneschilling

This comment has been minimized.

@anneschilling

This comment has been minimized.

@anneschilling anneschilling changed the title Implementation of weak and strong k-tableaux Implementation of weak k-tableaux Jun 19, 2013
@zabrocki
Copy link
Mannequin

zabrocki mannequin commented Jun 19, 2013

comment:4

Strong tableaux was moved into ticket #14776

@anneschilling
Copy link
Author

Reviewer: Mike Zabrocki, Anne Schilling

@anneschilling

This comment has been minimized.

@anneschilling
Copy link
Author

Dependencies: #14519

@tscrim
Copy link
Collaborator

tscrim commented Jul 13, 2013

comment:9

Hey Anne and Mike,

Thank you both for your work on this. However there are a few things I've noticed while glancing over the patch:

  • The inner bullet list on the INPUT: block for WeakTableau has one too many spaces, so it is overly indented. It needs to line up as follows:
- item 1

  - inner item a
  - inner item b

- item 2
  • k_charge_I() and k_charge_J() have the same docstring. In fact, it might be worthwhile to actually describe the algorithm.
  • I think it is worthwhile to have one abstract base class for all WeakTableau elements which has it's abstract methods defined. For example, you can also consolidate all of the more detailed explanations of the methods in there and link to them in their concrete classes. In particular, it because an easier/more straightforward to check to see if it's a WeakTableau and allow you an easy way to compare __eq__ between different representations of a weak tableau.
  • In the same vein, could we make it so that we can convert between different representations with parents? I.e. I'd like to do the following:
sage: T = WeakTableaux(3, [5,2,1], [1,1,1,1,1,1])
sage: T2 = WeakTableaux(3, [3,2,1], [1,1,1,1,1,1], representation='bounded')
sage: T2(T[0])
  • Could we have a method like T.representation('bounded') which returns the corresponding parent using a different representation. Same on the elements (which of course would call the respective to_* method, but it would make it consistent IMO)?
  • I don't see an easy way to go to a factorized permutation from the other 2 representations. Right now it seems like I have to call
WeakTableau_factorized_permutation.from_core_tableau().
  • I think the from_* (and quite possibly the to_*) methods maybe should be available in the parents rather than as classmethods for the element classes.
  • The .. SEEALSO:: block in k_charge() is not formatting correctly, so it doesn't link the methods.
  • Could you use the :arxiv:`1234.5678` autolink?
  • Remove self as an input (ex. in list_of_standard_cells())

Ack, that ended up being a longer list than I expected...

Best,

Travis

@zabrocki
Copy link
Mannequin

zabrocki mannequin commented Jul 13, 2013

Changed author from Anne Schilling, Mike Zabrocki to Anne Schilling

@zabrocki
Copy link
Mannequin

zabrocki mannequin commented Jul 13, 2013

Changed reviewer from Mike Zabrocki, Anne Schilling to Mike Zabrocki

@zabrocki zabrocki mannequin added s: needs work and removed s: needs review labels Jul 13, 2013
@anneschilling
Copy link
Author

comment:11

Hi Travis and Mike,

Thank you for your review comments. I incorporated them all, except for the input of the inner shape yet and the comments below:

  • In the same vein, could we make it so that we can convert between different representations with parents? I.e. I'd like to do the following:
sage: T = WeakTableaux(3, [5,2,1], [1,1,1,1,1,1])
sage: T2 = WeakTableaux(3, [3,2,1], [1,1,1,1,1,1], representation='bounded')
sage: T2(T[0])

We discussed coercing during design discussions at Sage Days 49 and currently decided against it. For now, it is easy to go between the various representations with the representation method. If we really need this feature, we can always add coercion later.

  • I think the from_* (and quite possibly the to_*) methods maybe should be available in the parents rather than as classmethods for the element classes.

This is what Nicolas suggested to me, so I will leave it as is.

Best,

Anne

@anneschilling
Copy link
Author

Changed reviewer from Mike Zabrocki to Mike Zabrocki, Travis Scrimshaw

@anneschilling
Copy link
Author

comment:13

Hi Mike,

Thanks for your review patch! I incorporated the changes. The new version now take as input

WeakTableaux(k, shape, weight, representation = 'core')

where shape is either a shape or a tuple of shapes in the skew case.

I hope everything looks ok now!

Anne

@anneschilling
Copy link
Author

Changed dependencies from #14519 to #14519, #14101

@anneschilling
Copy link
Author

comment:15

Hi Mike,

The updated patch incorporated all changes we discussed (by e-mail). It

  • checks that the weight, shape, etc of the element agrees with its parent
  • checks for semistandardness
  • implements latex and pretty printing methods
  • makes some changes Nicolas suggested

The semistandard checks and pretty printing rely on #14101, so I put this as a dependency.

Best,

Anne

@anneschilling
Copy link
Author

comment:16

Hi Mike,

I just posted a new version which incorporates your two doc review patches and fixes the bug in weights.

Anne

@anneschilling
Copy link
Author

comment:17

Fixed two more little issues that Mike found (spurious import of Tableau and innter_shape -> inner_shape).

Anne

@zabrocki zabrocki mannequin removed the s: needs review label Jul 21, 2013
@zabrocki zabrocki mannequin added the s: positive review label Jul 21, 2013
@zabrocki
Copy link
Mannequin

zabrocki mannequin commented Jul 21, 2013

comment:19

I'm happy and all tests pass.

@anneschilling
Copy link
Author

comment:20

Replying to @zabrocki:

I'm happy and all tests pass.

Great, thank you for the review! I just made height_of_restriced_subword a private method. Nothing else changed.

Anne

@zabrocki
Copy link
Mannequin

zabrocki mannequin commented Jul 21, 2013

comment:21

I reaffirm positive review.

-Mike

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Jul 21, 2013
@anneschilling
Copy link
Author

Attachment: trac_12250-ktableaux-as.patch.gz

@jdemeyer
Copy link

jdemeyer commented Aug 2, 2013

Changed dependencies from #14519, #14101 to #14519, #14101, #7983, #14772, #13589, #14907, #10630, #14143, #14015, #14516

@jdemeyer jdemeyer removed this from the sage-5.12 milestone Aug 2, 2013
@anneschilling anneschilling added this to the sage-5.12 milestone Aug 3, 2013
@jdemeyer
Copy link

jdemeyer commented Sep 2, 2013

Merged: sage-5.12.beta5

@anneschilling
Copy link
Author

comment:27

See #15444 for a follow-up on the k-charge implementation.

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

3 participants