Additional functionality for Farey symbols

[monien]

The additional functionality included in this patch are:

• various possibilities for the tessellation of the fundamental domain (Dedekind, coset, none)
• membership test via the Lang, Lim, Tan algorithm
• improved LaTeX output (via xymatrix)
• reduction to cusp
• identify cusp class

Added tests for LLT algorithm and reduction to cusp.

CC: @vbraun @loefflerd

Component: modular forms

Keywords: Farey symbol, arithmetic subgroups

Author: Hartmut Monien

Branch/Commit: u/mraum/ticket/15265 @ f24bf98

Reviewer: David Loeffler, Martin Raum

Issue created by migration from https://trac.sagemath.org/ticket/15265

[loefflerd]

comment:3

Pedantic comments: you need a proper commit message on the patch, or the release manager will complain; and there is a failing doctest in the `latex` method in congroup_gamma.py (typo?). I'm running a more thorough doctest cycle now.

[loefflerd]

Reviewer: David Loeffler

[loefflerd]

comment:4

OK, here's something a bit more important. The LaTeX display for Farey symbols doesn't work in the notebook: instead of rendering the Farey symbol it prints a little box with the raw LaTeX code in it. The problem is that the notebook uses MathJax for its latex rendering, and xypic is not supported by MathJax. I did warn you about this some while back.

Other than that the new code looks fine.

[loefflerd]

Work Issues: LaTeX display doesn't work

[monien]

comment:5

I do understand that the notebook (mathjax) currently does not support xymatrix. There are some attempts to make xymatrix also work for mathjax which would cure this problem. For the time being the situation is not very different from the one for using tikz with graphs. So the usual story:

latex.add_to_preamble('\usepackage[all]{xy}')
latex.mathjax_avoid_list('xymatrix'). 

Then

view(FareySymbol(Gamma0(23)), engine='pdflatex', tightpage=True)

does produce a latex view of the Farey symbol. I would hope that xymatrix would be included in in mathjax sooner than later. Please have a look at http://sonoisa.github.io/xyjax/xyjax.html. One could of course check wether the latex is produced in a notebook or
not but that might lead to inconsistencies for the user.

I'll fix the (minor) problems tomorrow. Thanks for the prompt review.

[monien]

comment:6

• Fixed typo in _latex_ in congroup.py.
• Added proper commit message

[loefflerd]

comment:7

I'm not sure that the comparison with using tikz for graphs is a fair one. There's clearly no hope of getting standard LaTeX to render graphs, so it's reasonable to require an extra package. On the other hand, one can produce a viable approximation to a Farey symbol without using any extra packages, as in my recent ticket #14971; this may not look quite so nice as your version, but it avoids these compatibility issues.

Can you add a doctest illustrating your changes to arithgroup_element.acton?

[monien]

comment:8

I have added two doctests to arithgroup_element.acton. I do understand your concern about the latex. The question here is:
Is the graph for the Farey symbol just some equation or a graph. I would opt for the latter.

I have modified the _latex_ of the Farey symbol so that it does work with sagemath cloud.
To illustrate that it is fairly easy to use the current version of the latex output (one thing which
has been bugging me for a long time is the extra brackets in the latex output in a sage worksheet) I included two examples
how to use it.

The only other alternative I see is to add yet another property to the Farey symbol like graph() or the like to keep the nice
latex format.

[monien]

latex as would produced by my new latex version

[monien]

Attachment: test latex for Farey symbols.tiff.gz

use in sage worksheet

[monien]

Attachment: test new Farey symbols -- Sage.pdf.gz

Attachment: farey.patch.gz

Farey symbol enhancement

[vbraun]

comment:9

Just so that I understand correctly, there are two potential LaTeX outputs, one with (here) and one without xy (at #14971)? How about you do

   if 'xymatrix' in latex.mathjax_avoid_list():
       output_with_xy()
   else:
       output_without_xy()

Since xy is in the default latex preamble we should probably add it to the avoid list by default, but that would be for another ticket...

[monien]

Branch: u/hmonien/ticket/15265

[monien]

comment:11

Added modified version of David's latex for FareySymbol.

[sagetrac-git]

Commit: 119c796

[sagetrac-git]

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

119c796

Version with modified latex for FareySymbol.

[sagetrac-mraum]

comment:13

We did some parallel work. Hope it's ok for you to base the review on my branch (to which I already made some small changes).

---

New commits:

9da738a	trac 15265: improvements for FareySymbol.
119c796	Version with modified latex for FareySymbol.

[sagetrac-mraum]

comment:14

Another try to change the attached commit.

---

New commits:

9da738a	trac 15265: improvements for FareySymbol.
119c796	Version with modified latex for FareySymbol.

[sagetrac-mraum]

Changed commit from 119c796 to 52108f0

[sagetrac-mraum]

Changed branch from u/hmonien/ticket/15265 to u/mraum/ticket/15265

[sagetrac-mraum]

comment:15

Sorry for that, I'm not yet used to the new Trac system. I'm now changing the branch.

[sagetrac-git]

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

248d2bc	trac 15265: Add some further examples and one test to `_acton_` of elements of arithmetic groups.
961251a	trac 15265: Minor changes to farey.cpp and farey.hpp.
d82a0b6	trac 15265: Minor changes to sl2z.cpp and sl2z.hpp.
7d78f7c	trac 15265: Minor changes to farey_symbol.pyx

[sagetrac-git]

Changed commit from 52108f0 to 7d78f7c

[sagetrac-mraum]

Changed reviewer from David Loeffler to David Loeffler, Martin Raum

[sagetrac-mraum]

comment:17

Hartmut, could you review the few changes that I made to your code? I have the following questions/comments, which you could help me with:

reference for rademacher_matrix in farey.cpp?
farey.cpp:FareySymbol(istream& is) why not "is >> *this" ?
farey.cpp:370 : it seems that x will always have length 1.
farey.cpp:129 I understand this is tempting, but PRETTY_FUNCTION is gcc only.
farey_symbol.pyx:plot rgbcolor option does not apply to Dedekind tesselation. Add one example each

I still need to test the code, but hopefully I'll have time for this tomorrow.

[sagetrac-git]

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

be4148a

trac 15265: Additional change to farey.cpp.

[sagetrac-git]

Changed commit from 7d78f7c to be4148a

[monien]

comment:19

Martin,

• the reference for the Rademacher matrix is http://www.jstor.org/stable/2371313 .
• what did you want to say about is >> *this?
• fair enough and true: len(x)==1 here
• PRETTY_FUNCTION is a leftover from testing
• true, what is a consistent naming scheme?
• I liked the format "sting" ;-)

all the changes seem to be okay. I think there was one more test for odd and even groups which try to remember ... .

[sagetrac-git]

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

f24bf98

trac 15265: various changes, finishing review.

[sagetrac-git]

Changed commit from be4148a to f24bf98

[sagetrac-mraum]

comment:21

Ok, I made all necessary changes. Can you have a look?

Just one, last question: Why do you actually use extern "C"? farey_symbols.pyx is translated to a C++ file, so that shouldn't be necessary.

[monien]

comment:22

This was added by somebody - not me - in some previous version of SAGE to have it working on some particular machine. Otherwise everything looks fine now.

[sagetrac-mraum]

Changed work issues from LaTeX display doesn't work to none

[sagetrac-mraum]

comment:23

It's not wrong, and so let's keep it so that we don't break any architecture.

[vbraun]

comment:25

See #15719 for a documentation issue