Ticket #14048 (closed defect: fixed)
Fix a bug in class_size for QuiverMutationType (in types F and G)
| Reported by: | hthomas | Owned by: | sage-combinat |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.7 |
| Component: | combinatorics | Keywords: | cluster algebras, class_size, exceptional types |
| Cc: | gmoose05, stumpc5 | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Christian Stump |
| Authors: | Hugh Thomas | Merged in: | sage-5.7.beta3 |
| Dependencies: | Stopgaps: |
Description
Jeroen ran "pyflakes" on the sage code, which detects certain kinds of problems (which can then be confirmed by testing).
sage: X=QuiverMutationType('F',4,(2,1))
sage: X.class_size()
...
NameError: global name 'twist' is not defined
Same issue for type G.
Attachments
-
trac_14048-fix-cluster-class_size-bug-ht.patch
(1.7 KB) -
added by hthomas 4 months ago.
Change History
comment:3 Changed 4 months ago by hthomas
It would also be fine with me if you want to fold this into one of the big cluster patches.
comment:4 Changed 4 months ago by gmoose05
Thanks for catching this Jeroen and Hugh!
Just to clarify, when you say "this wasn't the only problem in the code", does trac_14048-fix-cluster-class_size-bug-ht.patch appear to fix the issue that Jereon found or was there an additional issue that we had to deal with?
Also to update you: 10527 and 10538 are merged into 5.5 and 5.6.
Christian, Salvatore, and I are nearing completion of 13369 (perhaps by next week), and 13424 should be ready shortly after.
If the ticket really is just these couple lines, and I don't believe 13369 or 13424 alter quiver_mutation_type.py, we could double-check that this fix would be independent of our current work and then possibly give it a quick positive review.
Also, Christian and I will both be at ICERM Feb 11-15 so can discuss it there, if this ticket is still outstanding then.
Gregg
comment:5 Changed 4 months ago by hthomas
I fixed the issue Jeroen found and also the issue I found. So this ticket is ready to go and, so far as I know, makes class_size work correctly.
It sounds like the easiest thing might be if you can check it doesn't play badly with the new patches, and give it a green light.
cheers,
Hugh
Oh, also, turns out we were checking that twist be a tuple, and it is never a tuple, so this wasn't the only problem in the code!
Patch coming soon.