Opened 8 years ago
Closed 7 years ago
#16679 closed enhancement (fixed)
Plot the triangulation of a disk associated to a sineGordon Ysystem
Reported by:  Salvatore Stella  Owned by:  

Priority:  minor  Milestone:  sage6.10 
Component:  combinatorics  Keywords:  Ysystems, cluster algebras, triangulations 
Cc:  nakanisi@…  Merged in:  
Authors:  Salvatore Stella  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  371dbea (Commits, GitHub, GitLab)  Commit:  371dbea45fcb209e526a97df84e31864d28a1986 
Dependencies:  Stopgaps: 
Description
This code constructs the triangulation associated to a (reduced) sineGordon Ysystem as described in http://arxiv.org/abs/1212.6853
Change History (16)
comment:1 Changed 8 years ago by
Commit:  → c810471876572d0bfc7ddb62cb0abe89b64f3bdd 

comment:2 Changed 8 years ago by
Status:  new → needs_review 

comment:3 Changed 8 years ago by
Milestone:  sage6.3 → sage6.4 

comment:4 Changed 8 years ago by
Branch:  u/etn40ff/sine_gordon → public/ticket/16679 

Commit:  c810471876572d0bfc7ddb62cb0abe89b64f3bdd → 2f4a7539d65d4800d73e26caba8e6b6d801c7060 
Milestone:  sage6.4 → sage6.6 
comment:5 Changed 8 years ago by
A few things that need work from a quick lookover:
 You need to add docstrings and tests to
__init__
and_repr_
(and changed from__repr__
).  You should use
cached_method
instead of the_intervals
and_triangulation
workaround.
I will also try to find some time to take a more thorough lookthrough.
comment:6 Changed 8 years ago by
Commit:  2f4a7539d65d4800d73e26caba8e6b6d801c7060 → 4e2eea03f254d5574d283b17af37faa051380f9a 

Branch pushed to git repo; I updated commit sha1. New commits:
4e2eea0  trac #16679 use cached_method, reach 100% coverage

comment:7 Changed 8 years ago by
Thank you both for your input and sorry for the late reply. It look like Frédéric already took care of adding docstrings and replacing my workarounds with @cached_method. Is there anything else that should be done?
comment:8 Changed 8 years ago by
Commit:  4e2eea03f254d5574d283b17af37faa051380f9a → 81e911a5537af8e15a66de0a09c80bce2b41ebd7 

Branch pushed to git repo; I updated commit sha1. New commits:
81e911a  Removed more workarounds

comment:9 Changed 7 years ago by
Commit:  81e911a5537af8e15a66de0a09c80bce2b41ebd7 → f699370fd73dde0e66958c41534ca9544fd09132 

Branch pushed to git repo; I updated commit sha1. New commits:
b59b0bb  Implementation of the algorithm to plot the triangulation associated to a sineGordon Ysystem

65630bf  trac #16679 first cleanup pass

f9aaedb  trac #16679 use cached_method, reach 100% coverage

d5a6d2b  Removed more workarounds

f699370  Rebased to develop branch

comment:10 Changed 7 years ago by
this should use the existing arc plotting function (from sage.plot.arc import arc)
see #18307 for an example
or maybe even better, use the existing hyperbolic plot facilities
comment:11 Changed 7 years ago by
I assume you are talking about the parametric_plot in lines 519524 and 542546 (the other does not plot arcs).
If I replace them with sage.plot.arc.arc I then do not know how to plot the gray triangular regions. Right now, in lines 572576 I get a list of points out of three 'parametric_plot arcs' and then make a polygon from this list. Unfortunately I do not know how to get points from the output of sage.plot.arc.arc, nor if it is possible to shade a region not bounded by line segments.
Few considerations: sage.plot.arc.arc is extremely more efficient than the workaround with parametric_plot precisely because it does not have to compute points. Luckily the number of arcs that this routine plots is not big enough for this to be a (big) issue. Moreover, as you suggested, it would be nice to use the hyperbolic plot facilities already in sage but this would mean implementing two different logics (one for the unpunctured disk and one for the punctured disk) because the punctured disk, being a cover of the UHP, is not implemented as a hyperbolic model. I am not sure this would make the code more readable.
Any suggestion on how to address this?
comment:12 Changed 7 years ago by
Commit:  f699370fd73dde0e66958c41534ca9544fd09132 → 87060c0be66fe1667d7a5b16d3bad383804ec387 

Branch pushed to git repo; I updated commit sha1. New commits:
87060c0  Merge branch 'public/ticket/16679' into 6.10.b4

comment:13 Changed 7 years ago by
Milestone:  sage6.6 → sage6.10 

comment:14 Changed 7 years ago by
Commit:  87060c0be66fe1667d7a5b16d3bad383804ec387 → 371dbea45fcb209e526a97df84e31864d28a1986 

Branch pushed to git repo; I updated commit sha1. New commits:
371dbea  Merge branch 'public/ticket/16679' into 7.0.b3

comment:15 Changed 7 years ago by
Reviewers:  → Volker Braun 

Status:  needs_review → positive_review 
comment:16 Changed 7 years ago by
Branch:  public/ticket/16679 → 371dbea45fcb209e526a97df84e31864d28a1986 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Implementation of the algorithm to plot the triangulation associated to a sineGordon Ysystem