-
-
Notifications
You must be signed in to change notification settings - Fork 626
QSym: internal coproduct, Frobenius, lambda-of-monomials, documentation fixes #15094
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
Comments
Author: Darij Grinberg |
This comment has been minimized.
This comment has been minimized.
comment:1
Attachment: trac_15094-qsym_doc-dg.patch.gz |
Dependencies: #14775 |
comment:2
I have an initial review patch (not done yet) and without removing a few accented characters I was unable to compile the documentation. I cannot figure out why some of the doc strings do not appear in the documentation files (e.g. anything in I haven't played with the functionality, but just a comment about the math: If Adams / Frobenius is defined on QSym should Verschiebung be defined on NSym? Just a thought. |
comment:3
Thanks for looking into this! I copypasted the accents from the Windows host (no idea how to generate them in Ubuntu), so I'm not surprised they were causing issues; sorry for that. As for Yes, there are Verschiebungen on NSym, to be implemented in a later patch. |
comment:4
I don't think that identifiers need to be long to be unique. Maybe it is the hyphen, but the I will continue to look at the code. Please see if you can figure out why the documentation for |
comment:5
I've no idea; I'm not able to reasonably view the docs on my Ubuntu at all (neither html nor pdf which doesn't even compile) so I'm working more or less blindly as far as the markup goes. |
comment:6
Hi Darij, I added the dependency of #13505 because it modifies qsym.py and this patch will come after and so needs to be rebased. Can you fold the patches and rebase against it (and #14101)? I will upload a new version in a minute. The reason why the documentation does not appear is that The strangest thing I found in the documentation is that the word |
comment:7
Attachment: trac_15094_review-mz.patch.gz Thanks for the update. Commenting as I'm reading through it: Sorry for the trac syntax in the docstrings; that was stupid of me. I've changed
into
In contexts like
the period should be outside of the verbatim mode. I added a link to Gessel's paper in the reference list. Replaced "degree of the power series" by "total degree of the power series". "the product by the realization within the polynomial ring" replaced by "the product on the realization within the ring of power series". The next paragraph now reads
|
Attachment: trac_15094-not_rebased.patch.gz qfolded but not yet rebased. backup version in case mpatch fucks up |
comment:8
Rebased. Positive review then? |
comment:9
patchbot: apply trac_15094-rebased-QSym-dg.patch |
This comment has been minimized.
This comment has been minimized.
comment:11
I am getting 9 doctests failing Please check, but I think that you need to add the line
|
Attachment: trac_15094-rebased-QSym-dg.patch.gz qfolded & rebased |
comment:12
Oops! Thanks for catching this creepy issue (I see where it came from; it shows rebasing is not as simple as merging diffs). The tests (well, those in the ncsf_qsym subfolder) pass now. |
comment:13
I also checked and adding that line everything passes. |
comment:14
Thank you again! |
Reviewer: Mike Zabrocki |
Changed reviewer from Mike Zabrocki to Mike Zabrocki, Travis Scrimshaw |
comment:16
Attachment: trac_15094-review-ts.patch.gz I got some docbuilder warnings and there some doc formatting issues. Here's a review patch which fixes these as well as makes one notational change from |
This comment has been minimized.
This comment has been minimized.
comment:17
For patchbot: Apply: trac_15094-rebased-QSym-dg.patch, trac_15094-review-ts.patch |
Attachment: trac_15094-last-changes-dg.patch.gz |
comment:18
Nice changes; here's just a couple of typos fixed. I'm setting it to positive review, OK? For patchbot: Apply: trac_15094-rebased-QSym-dg.patch, trac_15094-review-ts.patch trac_15094-last-changes-dg.patch |
This comment has been minimized.
This comment has been minimized.
Merged: sage-5.13.beta0 |
The patch does the following:
Implement the internal coproduct on QSym, the ring of quasi-symmetric functions. (There is no reasonable internal product on QSym.)
Implement the Frobenius=Adams endomorphisms on QSym. (There seems to be no Verschiebung.)
Add a method that computes the lambda-ring operations at the monomial basis elements. This will be very useful later when we implement Hazewinkel's polynomial basis.
Fix errors in the docstrings in
sage/combinat/ncsf_qsym/qsym.py
. The fundamental basis was defined incorrectly. The coproduct was claimed to be inherited from the polynomial ring (which was wrong). The finitely-many-variables case was moved from the beginning to the end of the introduction because it is not implemented in Sage. Shuffles were replaced by stuffles in the definition of the product on the monomial basis.There are some obvious ways to go from here (corresponding changes on NSym, the Hazewinkel basis, possibly optimizing the dual immaculates etc.) but I am done for now.
The #14775 dependency is only because of a reference in the docstrings.
Apply:
Depends on #14775
Depends on #13505
CC: @sagetrac-sage-combinat @zabrocki @saliola @sagetrac-chrisjamesberg @jbandlow
Component: combinatorics
Keywords: sage-combinat, qsym, quasi-symmetric functions
Author: Darij Grinberg
Reviewer: Mike Zabrocki, Travis Scrimshaw
Merged: sage-5.13.beta0
Issue created by migration from https://trac.sagemath.org/ticket/15094
The text was updated successfully, but these errors were encountered: