Implementation of the Cyclic Sieving Phenomenon

[stumpc5]

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.

[stumpc5]

I added another possibility to give only the orbit sizes to obtain the polynomial P.

[chapoton]

Is there a reason why there are two patches here ? The buildbot gets confused.

Same thing for the patch Ticket #10065 on posets.

[stumpc5]

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.

[chapoton]

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.

[stumpc5]

Replying to chapoton:

Then the buildbot will be slightly more happy.

done, thanks...

[chapoton]

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..

[stumpc5]

Salut Fred --

Thanks for looking at this patch! I made the changed 1:1 according to your suggestions.

Best, Christian

[chapoton]

Hello, Christian. 

You did not answer my point 6. Do you really need QQ ?

Best, Fred

[stumpc5]

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?

[chapoton]

Replying to stumpc5:

Replying to chapoton:

You did not answer my point 6. Do you really need QQ ?

I use it when defining the polynomial: R = QQ[wiki:'q' q] Or did you think of doing that differently ?

What about using

R = ZZ['q']

instead ?

[stumpc5]

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

[chapoton]

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)

[jdemeyer]

Milestone sage-4.7.3 deleted