Opened 10 years ago

Closed 9 years ago

#13369 closed enhancement (fixed)

Implementation of the class ClusterSeed

Reported by: stumpc5 Owned by: sage-combinat
Priority: major Milestone: sage-5.8
Component: combinatorics Keywords: cluster algebra, quiver, days45
Cc: Merged in: sage-5.8.beta0
Authors: Christian Stump, Gregg Musiker Reviewers: Salvatore Stella
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #10538 Stopgaps:

Status badges

Description (last modified by gmoose05)

This class implements cluster seeds for skew-symmetrizable matrices.

The patch contains multiple examples.

sage: S = ClusterSeed(['A',3]); S
A seed for a cluster algebra of rank 3 of type ['A', 3]

Attachments (3)

trac_13369_cluster_seed.patch (37.4 KB) - added by stumpc5 9 years ago.
trac_13369-review.patch (12.5 KB) - added by stumpc5 9 years ago.
trac_13369-second_review.patch (12.2 KB) - added by stumpc5 9 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by stumpc5

  • Status changed from new to needs_review

comment:2 follow-up: Changed 10 years ago by gmoose05

  • trac_13369-cluster_seed-cs-NEW.patch is an exact copy of trac_13369-cluster_seed-cs.patch, except in quiver.py where the change to lines 97-100 now apply to lines 201-204 instead. (Also one typo fixed.)

This was done because trac_13369-cluster_seed-cs.patch no longer applies after recent changes to 10538, although it really was only this four line conflict.

  • More substantially, trac_13369-cluster-seed-gm.patch fixes compatibility issues with recent changes to 10538 and 10527. One can now apply all patches in 10527 in order, then all patches in 10538 in order, then the *two patches*

trac_13369-cluster_seed-cs-NEW.patch followed by trac_13369-cluster_seed-gm.patch,

and then the rebuilt sage passes all doc tests and appears to work correctly.

  • A more substantial review will be forthcoming.

comment:3 Changed 10 years ago by gmoose05

  • Description modified (diff)

comment:4 Changed 10 years ago by stumpc5

  • Description modified (diff)
  • Milestone changed from sage-5.3 to sage-5.4

comment:5 in reply to: ↑ 2 Changed 10 years ago by stumpc5

Replying to gmoose05:

I just renamed and uploaded the two relevant patches without any changes.

comment:6 Changed 10 years ago by gmoose05

Also, despite the patchbot's inability to apply the patches, it works fine with 5.3.rc1 on my machine (with the 10527 and 10538 patches applied first).

comment:7 Changed 10 years ago by stumpc5

Both patches rebased according to the new lazy import. No other changes.

comment:8 Changed 9 years ago by etn40ff

  • Reviewers set to Salvatore Stella

comment:9 Changed 9 years ago by stumpc5

Hi,

I updated my first patch (and uploaded Gregg's patch without changes). This resulted in some missing doctests which I then added in yet another patch. We also had some old things in the ClusterVariable class which I removed. If we need that back later, we can do it in another ticket...

Cheers, Christian

comment:10 Changed 9 years ago by stumpc5

  • Authors changed from Christian Stump to Christian Stump, Gregg Musiker
  • Description modified (diff)

Changed 9 years ago by stumpc5

comment:11 Changed 9 years ago by gmoose05

  • Description modified (diff)

Changed 9 years ago by stumpc5

Changed 9 years ago by stumpc5

comment:12 Changed 9 years ago by stumpc5

  • Status changed from needs_review to positive_review

comment:13 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.7 to sage-5.8

comment:14 Changed 9 years ago by etn40ff

  • Keywords days45 added

comment:15 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.8.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.