Opened 9 years ago

Closed 8 years ago

#14971 closed enhancement (fixed)

Better latex for Farey symbols

Reported by: davidloeffler Owned by:
Priority: major Milestone: sage-6.1
Component: modular forms Keywords: sd51
Cc: Merged in:
Authors: David Loeffler Reviewers: Martin Raum
Report Upstream: N/A Work issues:
Branch: u/mraum/ticket/14971 (Commits, GitHub, GitLab) Commit: 6d70fc410525d5e8ebe39db10f7ef91cbf663bf0
Dependencies: Stopgaps:

Status badges

Description

This patch adds a LaTeX rendering for Farey symbols (using the notation from Kurth and Long's paper, http://arxiv.org/abs/0710.1835).

Attachments (1)

trac_14971-farey_latex.patch (2.8 KB) - added by davidloeffler 9 years ago.
Patch against 5.11.beta3

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by davidloeffler

Patch against 5.11.beta3

comment:1 Changed 9 years ago by davidloeffler

Here's a patch. It also corrects the rendering of the fundamental domain in a certain corner case where we were previously plotting a fundamental domain that was too large.

comment:2 Changed 9 years ago by davidloeffler

  • Status changed from new to needs_review

comment:3 Changed 9 years ago by hmonien

Dear David, I have been working on a major overhaul of the Farey symbols including among other major improvements the latex code for Farey symbols e.g.

sage: f=FareySymbol?(Gamma0(23)) sage: latex(f) \xymatrix{& -\infty \ar@{-}@/_{1pc}/[r]_{1}& 0 \ar@{-}@/_{1pc}/[r]_{2}& \frac{1}{5} \ar@{-}@/_{1pc}/[r]_{3}& \frac{1}{4} \ar@{-}@/_{1pc}/[r]_{5}& \frac{1}{3} \ar@{-}@/_{1pc}/[r]_{3}& \frac{1}{2} \ar@{-}@/_{1pc}/[r]_{4}& \frac{2}{3} \ar@{-}@/_{1pc}/[r]_{2}& \frac{3}{4} \ar@{-}@/_{1pc}/[r]_{4}& \frac{4}{5} \ar@{-}@/_{1pc}/[r]_{5}& 1 \ar@{-}@/_{1pc}/[r]_{1}&\infty}

In my patch which I have to rework because the Farey symbol code in sage has been changed between 5.9 and 5.10 will include cusp_class and reduce_to_cusp. I have to remove some experimental features which are probably not useful for everybody.

So should I merge your changes into my patch?

comment:4 Changed 9 years ago by davidloeffler

Does that work in the Sage notebook? I don't know if the MathJax viewer for LaTeX formulae supports xypic.

Anyway, it looks like your patch will supersede this one here (I guess using xypic will give much nicer output than the rather simplistic approach based on \underbrace which my patch uses). So if your code is ready before anyone gets round to reviewing this one, we can close this ticket. But let's keep it open for now.

comment:5 Changed 8 years ago by mraum

  • Branch set to u/mraum/ticket/14971
  • Created changed from 07/25/13 19:18:48 to 07/25/13 19:18:48
  • Modified changed from 09/12/13 09:01:04 to 09/12/13 09:01:04

comment:6 Changed 8 years ago by mraum

  • Commit set to 6d70fc410525d5e8ebe39db10f7ef91cbf663bf0

I turned this into a git branch. I kept David as the author, even though I took the freedom not to convert the second part of the patch, which is not concerned with latex output).

I can give a positive review to the parts that I converted, but I need somebody to sign off that we do not take over the second part. First, it doesn't respect the "show_tesselation" option. Second, it leads to code duplication for the sake of what I think is a very little and insignificant speed up.


New commits:

6d70fc4trac 14971: Add LaTeX rendering for Farey symbols.

comment:7 Changed 8 years ago by davidloeffler

Hi Martin,

The second part of the patch, which you are proposing to reject, wasn't intended as a speedup: if I remember correctly (this was a while ago) the special-case code was there to correct a wrong output that was being returned in a specific corner case, the unique index 2 subgroup of PSL2Z. Anyway it indeed has nothing to do with latex formatting, so it should probably have been a separate ticket all along.

David

comment:8 Changed 8 years ago by mraum

Hi David:

That's good to hear. May I then propose that you give a positive review to leaving away the second part? I have reviewed and tested the rest of the ticket. (The patchbot doesn't seem to get the merging right. Any idea how to fix that, that is, let the patchbot know it is based on 6.0beta4?)

Please note that the index 2 subgroup is dealt with in Hartmut's updated FareySymbol?'s. I have made this ticket a dependency of Hartmut's to avoid having a Sage version which only has xymatrix output.

Martin

comment:9 Changed 8 years ago by davidloeffler

  • Status changed from needs_review to positive_review

OK, that sounds like the right way to go. I'm afraid I don't know how to fix the patchbot!

comment:10 Changed 8 years ago by vbraun

  • Reviewers set to Martin Raum

comment:11 Changed 8 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.