Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#1220 closed defect (fixed)

[with patch, positive review] ModularSymbols(GammaH)

Reported by: ifti Owned by: craigcitro
Priority: major Milestone: sage-3.1
Component: modular forms Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

On Nov 20, 2007 9:52 AM, Iftikhar Burhanuddin wrote:

Hi folks,

Gamma_H(N) are SL_2(Z) matrices where the lower left entry is congruent to 0 mod N, and the upper left and lower right entries are elements of a specified subgroup H of (Z/N)*.

Sage raises an exception when computing ModularSymbols? for the congruence subgroup Gamma_H(N) of weight 2. Is this a bug?

Yes. Report it on trac. This will actually probably be pretty hard to debug, but I will do it as soon as I get a chance. Modular symbols for Gamma_H are pretty subtle... In the meantime, keep in mind that you can compute most things by reducing to computing modular symbols with some character. In fact over CC at least modular symbols for Gamma_H(N) are the same as the direct sum over characters chi that vanish on H of modular symbols for chi. In particular, I think the following calculation shows that ModularSymbols?(Gamma_H(N)) for your particular example is just isomorphic to ModularSymbols?(Gamma_0(N)):

sage: G = GammaH(36, [13,19])
sage: G
Congruence Subgroup Gamma_H(36) with H generated by [13, 19]
sage: ModularSymbols(G, 2)
Traceback (most recent call last):
...
AssertionError
sage: X = G._list_of_elements_in_H(); X
[1, 7, 13, 19, 25, 31]
sage: euler_phi(36)
12
sage: H.<a,b> = DirichletGroup(36)
sage: for eps in H:
...       print eps, len(set([eps(w) for w in X]))
[1, 1] 1
[-1, 1] 2
[1, zeta6] 3
[-1, zeta6] 6
[1, zeta6 - 1] 3
[-1, zeta6 - 1] 6
[1, -1] 1
[-1, -1] 2
[1, -zeta6] 3
[-1, -zeta6] 6
[1, -zeta6 + 1] 3
[-1, -zeta6 + 1] 6
sage: eps = H([1,-1])  # vanishes on X
sage: ModularSymbols(eps)
Modular Symbols space of dimension 0 and level 36, weight 2, character
[1, -1], sign 0, over Rational Field

Notice that the dimension is 0....

William

Attachments (2)

trac-1220.patch (52.7 KB) - added by craigcitro 13 years ago.
trac-1220-pt2.patch (1.3 KB) - added by craigcitro 13 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 13 years ago by mabshoff

  • Milestone set to sage-2.8.14

comment:2 Changed 13 years ago by mabshoff

  • Owner changed from was to craigcitro

Still broken in Sage 3.0.3 and maybe a job for Craig Citro :)

Cheers,

Michael

comment:3 Changed 13 years ago by craigcitro

  • Status changed from new to assigned

Craig Citro to the rescue! I have a fix for this. The problem is that the _reduce_coset method on GammaH is broken. I'll clean up the fix and post it within a few days. (I'm just noting this so no one else spends time tracking this down.)

comment:4 Changed 13 years ago by mabshoff

Hi Craig,

any movement here? It has been a week ;)

Cheers,

Michael

comment:5 follow-up: Changed 13 years ago by craigcitro

Yep, and I'll get on it Sunday, when I'm back from my honeymoon.

comment:6 in reply to: ↑ 5 Changed 13 years ago by mabshoff

Replying to craigcitro:

Yep, and I'll get on it Sunday, when I'm back from my honeymoon.

Don't worry about it then. Have fun :)

Cheers,

Michael

comment:7 Changed 13 years ago by craigcitro

  • Milestone changed from sage-3.1.1 to sage-3.1
  • Summary changed from ModularSymbols(GammaH) to [with patch, needs review] ModularSymbols(GammaH)

Ok, the fix is attached, with a handful more. The patch fixes the bug, which is that the _reduce_coset method was broken for GammaH. I rewrote it instead of fixing it, and it's roughly the same speed for some cases, and much faster in others:

OLD:

sage: G = GammaH(99,[67])
sage: %timeit G._reduce_coset(77, -3)
10000 loops, best of 3: 42.1 µs per loop
sage: %timeit G._reduce_coset(11, -3)
10000 loops, best of 3: 36.9 µs per loop

NEW:

sage: G = GammaH(99,[67])
sage: %timeit G._reduce_coset(77, -3)
100000 loops, best of 3: 17.3 µs per loop
sage: %timeit G._reduce_coset(11, -3)
100000 loops, best of 3: 17.2 µs per loop

Also, because I was fixing up functions in modular/congroup.py, I got carried away and doctested all of congroup.py and congroup_element.py ... so this patch also fixes #2527.

All doctests in modular/ pass -- I didn't run anything beyond that.

Changed 13 years ago by craigcitro

comment:8 Changed 13 years ago by craigcitro

One side note: the patch is against 3.1.alpha0 -- I don't yet have an alpha1 build to test it against. I suspect there should be no issues, but I did touch modular/modsym/boundary.py, which I modified a lot in #3800. I think it shouldn't cause any issue (based on the specific changes), but just let me know if someone runs into merge troubles.

comment:9 Changed 13 years ago by mabshoff

It applies to 3.1.alpha1 with some slight fuzz:

mabshoff@sage:/scratch/mabshoff/release-cycle/sage-3.1.alpha2/devel/sage$ patch -p1 < trac_1220.patch 
patching file sage/modular/congroup.py
patching file sage/modular/congroup_element.py
patching file sage/modular/dims.py
patching file sage/modular/modform/ambient.py
patching file sage/modular/modform/constructor.py
patching file sage/modular/modform/space.py
patching file sage/modular/modsym/ambient.py
patching file sage/modular/modsym/boundary.py
Hunk #1 succeeded at 288 (offset 146 lines).
patching file sage/modular/modsym/manin_symbols.py
patching file sage/modular/modsym/modsym.py
patching file sage/rings/arith.py
Hunk #2 succeeded at 1319 (offset 17 lines).

Cheers,

Michael

comment:10 Changed 13 years ago by mabshoff

I am seeing two doctest failures:

mabshoff@sage:/scratch/mabshoff/release-cycle/sage-3.1.alpha2$ ./sage -t -long devel/sage/sage/modular/congroup.py
sage -t -long devel/sage/sage/modular/congroup.py
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.alpha2/tmp/congroup.py", line 103:
    sage: Gamma0(11).__hash__()
Expected:
    -545929996
Got:
    466678398374495476
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.alpha2/tmp/congroup.py", line 105:
    sage: Gamma1(11).__hash__()
Expected:
    -830809815
Got:
    4909638266971150633
**********************************************************************
1 items had failures:
   2 of   3 in __main__.example_5
***Test Failed*** 2 failures.
For whitespace errors, see the file /scratch/mabshoff/release-cycle/sage-3.1.alpha2/tmp/.doctest_congroup.py
         [3.4 s]
exit code: 1024

I know the first one is #random since cmp depends on memory location, I think for the second one we need #32 and #64 bit.

Cheers,

Michael

comment:11 Changed 13 years ago by mabshoff

Oops, the above is the "second" failure. The first one is"

age -t -long devel/sage/sage/modular/congroup_element.py   
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.alpha2/tmp/congroup_element.py", line 72:
    sage: x.__cmp__(3)
Expected:
    1
Got:
    -1
**********************************************************************

Sorry for the extra post.

Cheers,

Michael

Changed 13 years ago by craigcitro

comment:12 Changed 13 years ago by craigcitro

Yep, those are definitely issues with 32 vs. 64 bit. New patch fixes both of these doctests.

comment:13 Changed 13 years ago by cremona

  • Summary changed from [with patch, needs review] ModularSymbols(GammaH) to [with patch, with positive review but one issue remains] ModularSymbols(GammaH)

Wow, you have done an amazing job on this -- very impressive documentation as well.

The two patches apply completely cleanly to my 3.1.alpha1. (I think Michael's fuzz was with alpha2.)

You have included doctests showing that the original problem is solved.

I tested all in sage.modular and had one failure, which appears to be the one I already reported and Craig has sorted elsewhere:

sage -t  devel/sage-1220/sage/modular/modsym/tests.py       **********************************************************************
File "/home/john/sage-3.1.alpha1/tmp/tests.py", line 207:
    sage: sage.modular.modsym.tests.Test().random(1)
Exception raised:
    Traceback (most recent call last):
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest __main__.example_10[1]>", line 1, in <module>
        sage.modular.modsym.tests.Test().random(Integer(1))###line 207:
    sage: sage.modular.modsym.tests.Test().random(1)
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/tests.py", line 211, in random
        self.test("random", seconds)
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/tests.py", line 235, in test
        self._do(name)
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/tests.py", line 196, in _do
        Test.__dict__["test_%s"%name](self)
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/tests.py", line 333, in test_random
        Test.__dict__[name](self)
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/tests.py", line 264, in test_csnew_dimension
        V = M.cuspidal_submodule().new_submodule()
      File "/home/john/sage-3.1.alpha1/local/lib/python2.5/site-packages/sage/modular/modsym/ambient.py", line 896, in cuspidal_submodule
        assert d == S.dimension(), "According to dimension formulas the cuspidal subspace of \"%s\" has dimension %s; however, computing it using modular symbols we obtained %s, so there is a bug (please report!)."%(self, d, S.dimension())
    AssertionError: According to dimension formulas the cuspidal subspace of "Modular Symbols space of dimension 6 and level 12, weight 4, character [-1, -1], sign 1, over Rational Field" has dimension 4; however, computing it using modular symbols we obtained 2, so there is a bug (please report!).
**********************************************************************
1 items had failures:
   1 of   2 in __main__.example_10
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/john/sage-3.1.alpha1/tmp/.doctest_tests.py
	 [3.9 s]

Once that has been fixed the rest is ready to go.

comment:14 follow-up: Changed 13 years ago by mabshoff

Hi John,

that issue should be addressed with the patch at #3814. So if that fixes it I can merge the tickets from both patches.

Cheers,

Michael

comment:15 in reply to: ↑ 14 Changed 13 years ago by cremona

Replying to mabshoff:

Hi John,

that issue should be addressed with the patch at #3814. So if that fixes it I can merge the tickets from both patches.

I'll try that, but #3814 has no patch that I can see!

John

Cheers,

Michael

comment:16 follow-up: Changed 13 years ago by mabshoff

  • Summary changed from [with patch, with positive review but one issue remains] ModularSymbols(GammaH) to [with patch, positive review] ModularSymbols(GammaH)

Since this issue at question is unrelated to this code I am changing this to a positive review.

Cheers,

Michael

comment:17 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged both patches in Sage 3.1.alpha2

comment:18 Changed 13 years ago by was

I also skimmed this patch and it looks good..

comment:19 in reply to: ↑ 16 Changed 13 years ago by cremona

Replying to mabshoff:

Since this issue at question is unrelated to this code I am changing this to a positive review.

Cheers,

Michael

That's fine by me.

Note: See TracTickets for help on using tickets.