Compute Mutation Class for Cluster Algebra Seed or Cluster Quiver

[sagetrac-gmoose05]

This ticket contains additional features for computing mutation classes of ClusterSeeds and ClusterQuivers.

Depends on #13369

Component: combinatorics

Keywords: cluster algebra, quiver, days45

Author: Christian Stump, Gregg Musiker

Reviewer: Gregg Musiker, Salvatore Stella

Merged: sage-5.9.beta0

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

[sagetrac-gmoose05]

comment:2

Rebased slightly now that tickets 10527 and 10538 are in Sage. I will modify further over the next few weeks.

[sagetrac-gmoose05]

Changed author from Gregg Musiker, Christian Stump to Christian Stump

[sagetrac-gmoose05]

Reviewer: Gregg Musiker

[sagetrac-gmoose05]

comment:4

New upload: added documentation and examples for new commands.

Still to do:

1. Add a function _has_two_cycles

2. Fix documenation for, or possibly delete "old-methods"

3. More thorough check of code and trying to break functions or find corner cases.

[stumpc5]

comment:5

Replying to @sagetrac-gmoose05:

2. Fix documenation for, or possibly delete "old-methods"

I uploaded a new version where I fixed several minor things: the patch should now apply and should have 100% coverage. I moreover deleted several methods that we do not need for this ticket (we might need to get them back later for #13425).

Please download the newest version of the patch!

Cheers, Christian

[sagetrac-gmoose05]

comment:6

Just added the missing _has_two_cycles function.

Christian's recent changes seems to have solidified the documentation and coverage. (Although I don't see ClusterSeed or MutationClass.py commands showing up)

Pending 13369 and some additional testing to make sure no corner cases, should be ready to go.

[stumpc5]

comment:7

Here is a first review:

• we should add some tests that the computed class sizes coincide with the expected class sizes in QuiverMutationType

• the non-iter methods (mutation_class <-> mutation_class_iter) should also contain examples since these are the typical used methods

• l1318: "# runs forever without the mutation type recognition patch applied" we should make clear what this means (I don't even know currently).

• recheck that the file mutation_class.py is organized well.

• building the doctest and going through the docs.

Beside these, the patch looks good to me: the code is a little complicated but the methods are well-documented.

[sagetrac-gmoose05]

Changed author from Christian Stump to Christian Stump, Gregg Musiker

[Etn40ff]

Changed keywords from cluster algebra, quiver to cluster algebra, quiver, days45

[Etn40ff]

Changed reviewer from Gregg Musiker to Gregg Musiker, Salvatore Stella

[jdemeyer]

comment:14

You should not use

except:

without exceptions, otherwise you catch KeyboardInterrupt. Catch specific exceptions or

except StandardError:

if you really want "everything".

[stumpc5]

comment:15

Replying to @jdemeyer:

except StandardError:

fixed this issue (among others).

cheers, christian

[stumpc5]

comment:16

Is there anything left here? This question is in particular for Salvatore!

[sagetrac-gmoose05]

comment:17

I just posted a fourth review patch that

-slightly improves the documentation for up_to_equivalence, and

-starts depth counting at 0 when show_depth=True. This is most relevant for the iterator functions.

I uploaded this a second time (meaning to rewrite over the original) but forgot to check the "replace" box.

Consequently, I replaced the first file with a 216 byte empty patch.

Hopefully the patchbot will be happy with this. Christian, if you get a chance, feel free to delete the 216 byte patch.

Pending the patchbot and other further issues, I think this ticket is now ready for a positive review.

[stumpc5]

comment:18

Replying to @sagetrac-gmoose05:

I just posted a fourth review patch that

-slightly improves the documentation for up_to_equivalence, and

-starts depth counting at 0 when show_depth=True. This is most relevant for the iterator functions.

I am fine with all your changes, thanks! From my side, you can give it a positive review as soon as we get a green light from the patchbot.

Cheers, Christian

[stumpc5]

Attachment: trac_13424-mutation_classes.patch.gz

[jdemeyer]

Merged: sage-5.9.beta0