[with patch, with positive review] bug in modular symbols for GammaH subgroup

[was]

sage: ModularSymbols(GammaH(33,[1,2]),2).cuspidal_subspace()
Traceback (most recent call last):
...
KeyError: 11

[AlexGhitza]

I've done a bit of digging and gotten a bit closer to the source of the problem (which I think is actually in congroup.py):

sage: G = GammaH(6, [5])
sage: M = ModularSymbols(G, 2)
sage: B = M.boundary_space()
sage: bas = M.basis(); bas
((1,0), (3,2), (4,1))
sage: B(bas[0])
[Infinity] - [0]
sage: B(bas[2])
[0] - [-1/2]
sage: B(bas[1])
Traceback (most recent call last):
...
KeyError: 3

[craigcitro]

I'm attaching a fix. The fix also cleans up a few related functions. The problem was in the _coset_reduction_data functions -- specifically, one of the functions only generated data up to the square root of the level. However, in use, one needs more data -- at the very least, one needs to generate all data up to where the gcd of the term with N is at most square root of N. In that case, it seems easier to just generate all the data.

Added some doctests, though I'm not completely satisfied with my doctests: in particular, I couldn't come up with a doctest for _reduce_cusp that would have t == -1. I'd be happy if someone came up with one and added it.

I just noticed that this patch is actually in the tree where I'd already applied AlexGhitza?'s patch from #2995. Given that it's already been merged anyway, I'm just not going to worry about it.

[ncalexan]

There are some typos, namely specfically in at least two places.

I'm worried about the lack of t==-1 doctest as well, but this looks good to me anyway.

[AlexGhitza]

Two comments:

1. this patch is great; not only does it fix the current issue, but also fixes #2938. Awesome!

2. regarding t = -1: looking at the code, I notice that t = -1 is returned only if, towards the end, u > N_over_2; but before that u goes through a bunch of reductions modulo d, so u is at most d. On the other hand, d=gcd(v,N), so the only way this can be bigger than half of N is if it's equal to N, i.e. if v = N. Having run a lot of random examples, I don't think t is ever -1, but it would be nice to have a theoretical proof of this (maybe William or John know this, or maybe they know how to construct an example with t=-1).

Having said this, I think we should be grateful for point (1) and merge, and figure out point (2) later.

[craigcitro]

Added a small patch fixing the typos ncalexan pointed out. Also, I'm closing #2938 as fixed.

[mabshoff]

Merged in Sage 3.0.1.alpha1