Opened 8 years ago

Closed 7 years ago

#16679 closed enhancement (fixed)

Plot the triangulation of a disk associated to a sine-Gordon Y-system

Reported by: Salvatore Stella Owned by:
Priority: minor Milestone: sage-6.10
Component: combinatorics Keywords: Y-systems, 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:

Status badges

Description

This code constructs the triangulation associated to a (reduced) sine-Gordon Y-system as described in http://arxiv.org/abs/1212.6853

Change History (16)

comment:1 Changed 8 years ago by git

Commit: c810471876572d0bfc7ddb62cb0abe89b64f3bdd

Branch pushed to git repo; I updated commit sha1. New commits:

c810471Implementation of the algorithm to plot the triangulation associated to a sine-Gordon Y-system

comment:2 Changed 8 years ago by Salvatore Stella

Status: newneeds_review

comment:3 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:4 Changed 8 years ago by Frédéric Chapoton

Branch: u/etn40ff/sine_gordonpublic/ticket/16679
Commit: c810471876572d0bfc7ddb62cb0abe89b64f3bdd2f4a7539d65d4800d73e26caba8e6b6d801c7060
Milestone: sage-6.4sage-6.6

I have just had a quick look, and formatted doc and code.


New commits:

577d075Merge branch 'u/etn40ff/sine_gordon' into 6.6.b1
2f4a753trac #16679 first cleanup pass

comment:5 Changed 8 years ago by Travis Scrimshaw

A few things that need work from a quick look-over:

  • 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 work-around.

I will also try to find some time to take a more thorough look-through.

comment:6 Changed 8 years ago by git

Commit: 2f4a7539d65d4800d73e26caba8e6b6d801c70604e2eea03f254d5574d283b17af37faa051380f9a

Branch pushed to git repo; I updated commit sha1. New commits:

4e2eea0trac #16679 use cached_method, reach 100% coverage

comment:7 Changed 8 years ago by Salvatore Stella

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 work-arounds with @cached_method. Is there anything else that should be done?

comment:8 Changed 8 years ago by git

Commit: 4e2eea03f254d5574d283b17af37faa051380f9a81e911a5537af8e15a66de0a09c80bce2b41ebd7

Branch pushed to git repo; I updated commit sha1. New commits:

81e911aRemoved more workarounds

comment:9 Changed 7 years ago by git

Commit: 81e911a5537af8e15a66de0a09c80bce2b41ebd7f699370fd73dde0e66958c41534ca9544fd09132

Branch pushed to git repo; I updated commit sha1. New commits:

b59b0bbImplementation of the algorithm to plot the triangulation associated to a sine-Gordon Y-system
65630bftrac #16679 first cleanup pass
f9aaedbtrac #16679 use cached_method, reach 100% coverage
d5a6d2bRemoved more workarounds
f699370Rebased to develop branch

comment:10 Changed 7 years ago by Frédéric Chapoton

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

Last edited 7 years ago by Frédéric Chapoton (previous) (diff)

comment:11 Changed 7 years ago by Salvatore Stella

I assume you are talking about the parametric_plot in lines 519-524 and 542-546 (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 572-576 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 git

Commit: f699370fd73dde0e66958c41534ca9544fd0913287060c0be66fe1667d7a5b16d3bad383804ec387

Branch pushed to git repo; I updated commit sha1. New commits:

87060c0Merge branch 'public/ticket/16679' into 6.10.b4

comment:13 Changed 7 years ago by Frédéric Chapoton

Milestone: sage-6.6sage-6.10

comment:14 Changed 7 years ago by git

Commit: 87060c0be66fe1667d7a5b16d3bad383804ec387371dbea45fcb209e526a97df84e31864d28a1986

Branch pushed to git repo; I updated commit sha1. New commits:

371dbeaMerge branch 'public/ticket/16679' into 7.0.b3

comment:15 Changed 7 years ago by Volker Braun

Reviewers: Volker Braun
Status: needs_reviewpositive_review

comment:16 Changed 7 years ago by Volker Braun

Branch: public/ticket/16679371dbea45fcb209e526a97df84e31864d28a1986
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.