#10155 closed enhancement (fixed)
Implementation of the Cyclic Sieving Phenomenon
Reported by: | Christian Stump | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-4.8 |
Component: | combinatorics | Keywords: | Cyclic Sieving Phenomenon |
Cc: | Merged in: | sage-4.8.alpha0 | |
Authors: | Christian Stump | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This patch implements the Cyclic Sieving Phenomenon (CSP) as described in
Reiner, Stanton, White - The cyclic sieving phenomenon, JCTA108 (2004)
Given a finite set S and a cyclic action cyc_act on S, the method CyclicSievingPolynomial
( S, cyc_act ) returns the unique polynomial P of order < n such that the triple ( S, cyc_act, P ) exhibits the CSP. The method CyclicSievingCheck
( S, cyc_act, P ) checks if this triple exhibits the CSP.
Attachments (1)
Change History (20)
comment:1 Changed 12 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:3 follow-up: 4 Changed 12 years ago by
Is there a reason why there are two patches here ? The buildbot gets confused.
Same thing for the patch Ticket #10065 on posets.
comment:4 Changed 12 years ago by
Replying to chapoton:
Is there a reason why there are two patches here ? The buildbot gets confused.
Same thing for the patch Ticket #10065 on posets.
I can override only files with the same name and not delete any (is that true?). So when the name changes, or if I forget to mark the box, the file stays there forever. I am happy to learn about a way to get around that.
For both tickets, only the youngest file contains the newest version of the patch.
comment:5 follow-up: 6 Changed 12 years ago by
Owner: | Sage Combinat CC user deleted |
---|
You should modify the header of your patch : add something like
#10155 Implementation of the Cyclic Sieving Phenomenon
Then the buildbot will be slightly more happy.
comment:6 Changed 12 years ago by
comment:7 Changed 12 years ago by
Milestone: | → sage-4.7.1 |
---|
comment:8 Changed 11 years ago by
Milestone: | sage-4.7.1 → sage-4.7.2 |
---|
comment:9 Changed 11 years ago by
Status: | needs_review → needs_work |
---|
This seems good enough. I will soon give a positive review if minor corrections are made.
1) at start of the patch, before AUTHORS, there is a broken (not complete) sentence.
2) I am not happy with the name of the procedures.
What about CyclicSieving_find and CyclicSieving_test ?
or CyclicSievingPolynomial? and CyclicSievingCheck? ?
or some mix of that..
3) You could use R.gen() instead of R.gens()[0] (at least twice)
4) in CyclicSieving?, you can use keys to compute n, instead of calling .keys() again
5) in orbit_decomposition, there misses the OUTPUT: a list of lists, the orbits under cyc_act acting on L
6) why do you need to import QQ ? because of q-analogues ? coercion may work with ZZ, maybe..
Changed 11 years ago by
Attachment: | trac_10155-cyclic_sieving_phenomenon-cs.patch added |
---|
comment:10 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
Salut Fred --
Thanks for looking at this patch! I made the changed 1:1 according to your suggestions.
Best, Christian
comment:11 follow-up: 12 Changed 11 years ago by
Hello, Christian.
You did not answer my point 6. Do you really need QQ ?
Best, Fred
comment:12 follow-up: 13 Changed 11 years ago by
Replying to chapoton:
You did not answer my point 6. Do you really need QQ ?
I use it when defining the polynomial:
R = QQ['q']
Or did you think of doing that differently?
comment:13 follow-up: 14 Changed 11 years ago by
comment:14 Changed 11 years ago by
Replying to chapoton:
Replying to stumpc5:
Replying to chapoton:
What about using
R = ZZ['q']
I don't know why, but the mod operation on R(f) does not work in that case. I had a quick look but didn't see the reason right away...
As the QQ doesn't really do anything negative (or does it?), I think we should just leave it.
Best, Christian
comment:15 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Reviewers: | → Frédéric Chapoton |
Status: | needs_review → positive_review |
ok, I agree that this is a minor point, and I give a positive review. (It seems that the patchbot is currently useless, so I do not require a green light from the bot)
comment:16 Changed 11 years ago by
Milestone: | sage-4.7.2 → sage-4.7.3 |
---|
comment:17 Changed 11 years ago by
Merged in: | → sage-4.7.3.alpha0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:19 Changed 11 years ago by
Merged in: | sage-4.7.3.alpha0 → sage-4.8.alpha0 |
---|---|
Milestone: | → sage-4.8 |
I added another possibility to give only the orbit sizes to obtain the polynomial P.