#14141: Implementation of Knutson-Tao puzzles
----------------------------------------------------------------------+-----
Reporter: aschilling |
Owner: sage-combinat
Type: enhancement |
Status: needs_review
Priority: major |
Milestone: sage-5.10
Component: combinatorics |
Resolution:
Keywords: puzzles, Littlewood-Richardson coefficiens, days45 | Work
issues:
Report Upstream: N/A |
Reviewers: Allen Knutson, Liz Beazley, Ed Richmond
Authors: Franco Saliola, Avinash Dalal, Anne Schilling |
Merged in:
Dependencies: |
Stopgaps:
----------------------------------------------------------------------+-----
Comment (by saliola):
General comments on the patch:
- This patch has not been rebased: it fails to apply on 5.9.beta5 (and on
5.9.rc0 according to the patchbot).
- All tests pass on 5.9.beta5.
- some methods are missing {{{INPUT}}} blocks, some do not describe all
the possible input arguments, some do not test all the possible input
options
Comments on the interface:
- I don't think that {{{PuzzlePiece, PuzzlePieces, PuzzleSolver}}} should
be imported into the global namespace. These names are too generic to
apply to only KT puzzles.
- Is it expected that the average user will create new puzzle pieces? or
is it that {{{PuzzleSolver}}} will be the most used command?
- I think it would be preferable to have one entry point to the module,
from which users import what they need.
- And this entry point should have appropriate documentation describing
all the features (kind of like the current documentation to the module,
but with some enhancements).
- Perhaps {{{KnutsonTaoPuzzleSolver}}}?
Comments on documentation to the module (top of the file):
- If I do {{{import knutson_tao_puzzles}}} followed by
{{{knutson_tao_puzzles?}}}, I see {{{1/0 0/1}}} as a puzzle piece. I
don't know what this piece is. Is this a problem with the documentation
rendering system?
- The string representations should be explained: most people won't know
what {{{1/0 0\1}}} means.
- The {{{TODO}}} section describes a possible speed up. Isn't this what
{{{_fill_strip}}} and {{{_fill_puzzle_by_strips}}} does?
Comments on {{{PartialPuzzle}}}:
- {{{LR_tab_from_puzzle}}} is not a good name. Perhaps just call it
{{{skew_lr_tableau}}}.
- {{{plot_thru_puzzle}}} is not a good name. {{{thru}}} is not an English
word.
- the error from comment 7 above should be included as a doctest.
- {{{assert}}} should be avoided.
- {{{partition_from_string}}}: I don't know what this method does. The
documentation is not descriptive. What is the "string data" of a
partition? If this is a standard partition constructor, then should it be
in {{{Partition}}} instead?
Comments on {{{PuzzleSolver}}}:
- "Initialize the puzzle solver" is not a sufficient 1-line description
since it doesn't mention the kind of puzzles dealt with.
- there is an option called 'maxLetter' in the INPUT section, but the init
doesn't take maxLetter as an argument ... oh wait, it's in
{{{__classcall_private__}}}. Why is {{{__classcall_private__}}} needed
here?
- there should be doctests that show how to use "maxLetter"
- The output is not a function (but instances of the class happen to be
callable).
- several methods are missing the mandatory 1-line string documentation
and instead just include examples / tests.
Comments on {{{SchubertCalculusAlgebra}}}:
- should this really be in this module? it seems to experimental (there is
also some commented out code that shouldn't be included)
- it was implemented as a proof of concept and has no special features;
perhaps it should be deleted and further developed in a separate patch?
- in the doctest for {{{product_on_basis}}}, the base ring of the algebra
is {{{QQ}}}, but the coefficients belong to a different ring:
{{{
sage: from sage.combinat.knutson_tao_puzzles import
SchubertCalculusAlgebra
sage: Sc = SchubertCalculusAlgebra(QQ, (3,3)).basis()
sage: x = Sc[Word('001011')] * Sc[Word('010101')]
sage: x.parent().base_ring()
Rational Field
sage: x.coefficients()[0].parent()
Multivariate Polynomial Ring in y0, y1, y2, y3, y4, y5, y6 over Integer
Ring
}}}
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14141#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.