search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

Implement Eulerian (quasi)symmetric functions #15454

Closed tscrim closed 10 years ago

tscrim commented 10 years ago

Implement the Eulerian (quasi)symmetric functions as defined by Shareshian and Wachs in http://arxiv.org/abs/0812.0764.

CC: @sagetrac-sage-combinat @anneschilling @darijgr

Component: combinatorics

Keywords: Eulerian sf, qsym

Author: Travis Scrimshaw

Branch/Commit: public/combinat/sf/eulerian @ ef45d4d

Reviewer: Darij Grinberg

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

tscrim commented 10 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-Implement the Eulerian (quasi)symmetric functions as defined by Shareshian and Wachs.
+Implement the Eulerian (quasi)symmetric functions as defined by Shareshian and Wachs in http://arxiv.org/abs/0812.0764.
tscrim commented 10 years ago
comment:2

Added the reference.

darijgr commented 10 years ago
comment:3

Interesting -- these were new to me, despite the paper being in my algebra folder...

What is the rationale behind replacing sage.combinat.partition.Partitions() by _Partitions? Speed? (And why only sometimes?)

EDIT: "permtutation", "excedences". Ok, the latter might need some explanation. Dictionaries say that "exceedences" and "exceedance" are both right, the latter form apparently being the standard one. Combinatorialists seem to use either "exceedance" or "excedance". Noone seems to use "excedence".

EDIT2: I find this ambiguous: `i is a descent and i is not an excedence or i + 1 is,`.

tscrim commented 10 years ago
comment:4

Replying to @darijgr:

Interesting -- these were new to me, despite the paper being in my algebra folder...

What is the rationale behind replacing sage.combinat.partition.Partitions() by _Partitions? Speed? (And why only sometimes?)

This was something introduced in #13605 since the set of all partitions is used all over the place and the UniqueRepr. calls do have some effect on speed. Plus this means it's never garbage collected.

EDIT: "permtutation", "excedences". Ok, the latter might need some explanation. Dictionaries say that "exceedences" and "exceedance" are both right, the latter form apparently being the standard one. Combinatorialists seem to use either "exceedance" or "excedance". Noone seems to use "excedence".

Hmmm...curious. I followed what is in permutation.py: the method weak_excedences(). Since this is just in the docstring, I think it'd be okay to use "excedance" to match the reference.

EDIT2: I find this ambiguous: `i is a descent and i is not an excedence or i + 1 is,`.

That was the least ambiguous way I could come up with. The parenthesis should be around the "or" but English syntax doesn't allow that. Oh, here's a thought:

`i` is a descent, and given that `i` is not an excedance or `i + 1` is,

And because I can't resist the (bad) pun: English is not a language decided around around logic.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from cd66c64 to 2fb942a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

[2fb942a](https://github.com/sagemath/sagetrac-mirror/commit/2fb942a)rest of the review
[d14940a](https://github.com/sagemath/sagetrac-mirror/commit/d14940a)the jackhammer approach to fixing the definition of dex
[de00f2e](https://github.com/sagemath/sagetrac-mirror/commit/de00f2e)Merge branch 'master' into eulerian
[881b3c5](https://github.com/sagemath/sagetrac-mirror/commit/881b3c5)spelling according to Shareshian-Wachs
darijgr commented 10 years ago
comment:6

Hi Travis,

feel free to set this to positive_review if you're happy with the changes. Nice work!

Best regards,

Darij

tscrim commented 10 years ago
comment:8

I let this slip away. Positive review. Thanks Darij.

tscrim commented 10 years ago

Reviewer: Darij Grinberg

vbraun commented 10 years ago
comment:9

Conflicts with #15408, please fix.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

6b05e43undo a change to prevent a conflict
cd4470btransferred functions from work in progress
6302b95some doc changes
c358c00noticed a missing article 'a' in a sentence
13b9b6ddocstrings reviewed
3c4ce5bsome doc changes II, for some reason
fe67878Franco's suggestions
6a1ea5awarnings added
a1a3000copypaste oops
3c7cb8aMerge branch 'public/combinat/15408/zabrocki/inner_plethysm' of trac.sagemath.org:sage into inner_plethysm_true
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from 2fb942a to ef45d4d

darijgr commented 10 years ago
comment:11

I've done the merge with some manual work (this ticket was editing a piece of code which was being completely replaced by #15408; I've removed this edit). Since both tickets are positive_reviewed, I'm taking the liberty to set this one to positive_review as well.

But here's something creepy. When I tried merging them and resolving conflicts the git way, my src/sage/combinat/sf/sfa.py became this: https://dl.dropboxusercontent.com/u/83265276/weird_stuff.py Search for the "<<<<<<<" and pleeeease try to explain me why code suddenly jumped into the docstring. Can't I trust git merge anymore?

tscrim commented 10 years ago
comment:12

It just means the merge got a little confused and didn't do the best job using the "recursive" strategy -- there are others that might have yielded better results. IMO using git mergetool (with meld for me) is easier/cleaner than true manual conflict editing.

tscrim commented 10 years ago
comment:13

Nevertheless, thanks Darij for doing the merge.