Implement the classical Hall algebra and polynomials

[tscrim]

Implements the classical Hall algebra and the corresponding Hall polynomials.

Apply:

• attachment: trac_15311-hall_algebras-ts.patch​

Depends on #15305

CC: @amritanshu-prasad @darijgr

Component: algebra

Keywords: Hall algebra polynomials

Author: Travis Scrimshaw

Reviewer: Darij Grinberg

Merged: sage-5.13.beta4

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

[darijgr]

comment:3

Review patch uploaded. I've got only one issue I'd like you to fix (unless it's intention), and that's the fact that the Hall algebra doesn't coerce to the symmetric functions until you call the HallAlgebraMonomials basis (because only the latter basis activates the coercions).

Comments on my changes:

• Your patch contained some coercion fu in the coproduct method, where you took an element of the tensor square of one basis and coerced it into the tensor square of another. This doesn't work in the current version of Sage, not even with Axioms and more functorial constructions #10963 applied (therefore the failing doctests). I have replaced this by a manual implementation. Is this fixed on the combinat queue?

• I prevented the coercion to the symmetric functions from appearing unless q is invertible. Is this unnecessarily restrictive? (The map does involve division by q.)

• Your scalar product functions had a q argument which seems pointless to me (the q is already an attribute of the parent, and I don't think you want to allow a different q in the scalar product -- if you do, you are doing it wrong). I removed it.

Nice work!

for the patchbot:

apply trac_15311-hall_algebras-ts.patch​ trac_15311-rev-dg.patch

[tscrim]

Dependencies: #15305

[tscrim]

comment:5

Replying to @darijgr:

Review patch uploaded. I've got only one issue I'd like you to fix (unless it's intention), and that's the fact that the Hall algebra doesn't coerce to the symmetric functions until you call the HallAlgebraMonomials basis (because only the latter basis activates the coercions).

I'll fix that in the next day or so.

Comments on my changes:

• Your patch contained some coercion fu in the coproduct method, where you took an element of the tensor square of one basis and coerced it into the tensor square of another. This doesn't work in the current version of Sage, not even with Axioms and more functorial constructions #10963 applied (therefore the failing doctests). I have replaced this by a manual implementation. Is this fixed on the combinat queue?

Ah, sorry I forgot the #15305 dependency which does this.

• I prevented the coercion to the symmetric functions from appearing unless q is invertible. Is this unnecessarily restrictive? (The map does involve division by q.)

I believe it makes the scalar product agree with the HL scalar product... I saw something that this made the map "nice", but I don't think it was in Schiffmann.

• Your scalar product functions had a q argument which seems pointless to me (the q is already an attribute of the parent, and I don't think you want to allow a different q in the scalar product -- if you do, you are doing it wrong). I removed it.

Fair enough.

Nice work!

Thanks, and thank you for reviewing this! I'll post a new version with your changes folded in once I've fixed the coercion issue.

See you soon,

Travis

[darijgr]

comment:6

BTW unless you married someone called Scirmshaw, there is one more misspelling of your name than what I caught :) (and many more in #15300)

[tscrim]

comment:7

I was trying out new spellings of my names, and then copy/paste fun! XP

So I've folded in your review patch and made the following changes:

• Fixed a bug so computations work for prime powers (I forgot to pass self._q to hall_polynomial()). This uncovered another problem, that [univariate] Laurent polynomials don't divide like the should (see Implement univariate Laurent polynomial ring & elements #11726), so I changed all of the tests to use the fraction field.
• Fixed the coercion.
• Made HallAlgebra lazily imported.
• Improved the doc and added some more doctests.

I didn't want to make this depend on #11726 since functionally it will work when Laurent polynomials do the division like they should (although it is horrendously slow with the current #11726 patch).

For patchbot:

Apply: trac_15311-hall_algebra-ts.patch

[darijgr]

comment:8

Did you actually do these changes? Because my edits aren't there and I still don't see HallAlgebra lazily imported. Looks like phases hit you again? (BTW, are there any good rules on when to import lazily vs. regularly, and how to import in general? Like, is it better to import ZZ from sage.rings.all, or rather define ZZ = IntegerRing() after importing IntegerRing from sage.rings.integer_ring?)

[tscrim]

comment:9

Ah sorry, I upgraded to beta2 but uploaded from the old one.

Since this is a niche thing IMO (wrt the rest of Sage), it's worthwhile it lazily import it to help keep startup time down. As far as the ZZ import, it just makes things easier for me to remember; I don't think there's an explicit difference other than having extra unneeded (in a loose sense of the word) variables running around.

For patchbot:

Apply: trac_15311-hall_algebra-ts.patch

[darijgr]

comment:10

Sorry, I forgot about specializations. Here is a fix for another thing I failed to notice: the q=0 case. If you like it, please set to positive_review. Or should I except another exception? (I don't really know what errors q**-1 can cause.)

for the patchbot:

apply trac_15311-hall_algebras-ts.patch​ trac_15311-rev-dg.patch

[tscrim]

comment:11

I believe the correct syntax is except (ZeroDivisionErorr, NotImplementedError):. Once this change is made, then you can set it to positive review. Thanks Darij.

[darijgr]

comment:12

I've taken a different solution, seeing that I didn't catch ValueError which too appears when trying to invert some things. I actually think this is one of the cases where an unqualified except is legit. If you're OK with this, please set to positive_review.

[tscrim]

comment:13

You should use except StandardError:, that way it doesn't catch things like keyboard interrupts and other more serious system errors.

[darijgr]

comment:14

Thank you! Positive review?

[tscrim]

Reviewer: Darij Grinberg

[tscrim]

comment:15

Yep. Thanks Darij.

[jdemeyer]

comment:18

The PDF doesn't build:

! Emergency stop.
<to be read again> 
                   $
l.5618 unity of this algebra is $I_\empty$
                                          .
!  ==> Fatal error occurred, no output PDF file produced!

[tscrim]

comment:19

Fixed (and I folded in the last review patch).

Output written on algebras.pdf (170 pages, 799758 bytes).
Transcript written on algebras.log.
Build finished.  The built documents can be found in /home/travis/sage/src/doc/output/pdf/en/reference/algebras

For patchbot:

Apply: trac_15311-hall_algebras-ts.patch​

[jdemeyer]

comment:20

The docbuilder reports:

dochtml.log:[algebras ] None:33: WARNING: citation not found: Schiffmann

[tscrim]

comment:21

Jeroen, I though references were global? The reference is defined in sage/combinat/hall_polynomial.py (lines 129-130):

    .. [Schiffmann] Oliver Schiffmann. *Lectures on Hall algebras*.
       :arxiv:`0611617v2`.

I'm running a full docbuild right now to check.

[tscrim]

comment:22

Attachment: trac_15311-hall_algebras-ts.patch.gz

...Or the hall_polynomial.py file was not included in the documentation. Sorry. Fixed.

For patchbot:

Apply: trac_15311-hall_algebras-ts.patch​

[jdemeyer]

comment:23

Documentation works now...

[jdemeyer]

Merged: sage-5.13.beta4