Opened 3 years ago
Last modified 2 years ago
#28176 closed defect
Fix few bugs in ClusterAlgebra discovered while working on #26771 — at Version 3
Reported by: | etn40ff | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.9 |
Component: | algebra | Keywords: | ClusterAlgebras |
Cc: | tscrim, gmoose05, stumpc5, gh-EBanaian, drupel, gh-kelleye | Merged in: | |
Authors: | Salvatore Stella | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/etn40ff/28176 (Commits, GitHub, GitLab) | Commit: | d3cd59d86bf16c8414d36cdf5d95d2c5abd7d07f |
Dependencies: | Stopgaps: |
Description (last modified by )
As pointed out by the people at #26771, ClusterAlgebra
has few bugs.
- Default arguments do not play nicely with
UniqueRepresentation
sage: A1 = ClusterAlgebra(['A',2]) sage: A2 = ClusterAlgebra(['A',2], cluster_variable_prefix='x') sage: A1 is A2 False
mutate_initial
keeps the same variable names; In theory this is not a problem but may create confusion. For example suppose we are looking at the Grassmannian of planes in a 4 dimensional vector space. Then the associate cluster algebra is
sage: A = ClusterAlgebra( matrix(5,[0,1,-1,1,-1]), cluster_variable_names=['p13'], coefficient_names=['p12','p23','p34','p41'], scalars=QQbar); A A Cluster Algebra with cluster variable p13 and coefficients p12, p23, p34, p41 over Algebraic Field
Swapping out
p13
from the initial seed should givep24
or, at the very least *not*p13
. Gettingp24
requires some understanding of what the ring in question really is that goes beyond the scope of this class. Currently we get
sage: A.mutate_initial(0) A Cluster Algebra with cluster variable p13 and coefficients p12, p23, p34, p41 over Algebraic Field
mutate_initial
does not compute all the required F-polynomials and it does not enforce the fact that they are not rational expressions
sage: A1 = ClusterAlgebra(['A',[2,1],1]) sage: A2 = A1.mutate_initial([0,1,0]) sage: len(A2.g_vectors_so_far()) == len(A2.F_polynomials_so_far()) False sage: all(parent(f) == A2._U for f in A2.F_polynomials_so_far()) False
- finally
mutate_initial
does not enforce the fact that initial g-vectors belong to the initial cluster but rather prepend the performed mutation sequence to all the paths already known.
sage: A2.find_g_vector((0,0,1)) [0, 1, 0]
This, while technically not false, may generate confusion.
_mutated_F
is doing/
rather than//
resulting in slower computations and parent issues.
This ticket deals with all these issues.
Change History (3)
comment:1 Changed 3 years ago by
- Branch set to u/etn40ff/28176
- Cc tscrim gmoose05 stumpc5 gh-EBanaian drupel gh-kelleye added
- Commit set to d3cd59d86bf16c8414d36cdf5d95d2c5abd7d07f
- Component changed from PLEASE CHANGE to algebra
- Status changed from new to needs_review
comment:2 Changed 3 years ago by
bot is not happy:
sage -t --long src/sage/algebras/cluster_algebra.py # Tab character found
comment:3 Changed 3 years ago by
- Description modified (diff)
Oops. Fixed, sorry Frédéric!
With the occasion I also got some dramatic speedup in mutate_initial
by not
mutating F-polynomials a quadratic number of times.
Also I changed a division into a polynomial division in _mutated_F
; this
enforces F-polynomials to live in the correct ambient and yields a decent
speedup in mutate
.
New commits:
Fix 28176