Opened 9 years ago

Closed 8 years ago

#10453 closed defect (fixed)

Problem with old submodule

Reported by: davidloeffler Owned by: craigcitro
Priority: major Milestone: sage-4.7.2
Component: modular forms Keywords:
Cc: Merged in: sage-4.7.2.alpha3
Authors: David Loeffler Reviewers: Johan Bosman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

Weird things happen if you try to compute the old submodule of a Gamma1 modular forms space. E.g.

sage: CuspForms(Gamma1(11), 2).old_submodule() # crashes

This turns out to be because:

sage: ModularForms(Gamma1(11), 2).hecke_module_of_level(1).hecke_module_of_level(11)
Modular Forms space of dimension 2 for Congruence Subgroup Gamma0(11) of weight 2 over Rational Field

so the degeneracy level-raising map is getting computed for the wrong space!

Prerequisite for #11601. (Note that #11601 depends on a bunch of other stuff as well, but this ticket is independent of any of the other patches in the series.)


Apply trac_10453_gamma1_old_submodule.patch to the Sage library.

Attachments (1)

trac_10453_gamma1_old_submodule.patch (24.3 KB) - added by davidloeffler 9 years ago.
patch against 4.6.1.alpha3 + #8716

Download all attachments as: .zip

Change History (11)

comment:1 Changed 9 years ago by davidloeffler

  • Description modified (diff)

Changed 9 years ago by davidloeffler

patch against 4.6.1.alpha3 + #8716

comment:2 Changed 9 years ago by davidloeffler

  • Authors set to David Loeffler
  • Status changed from new to needs_review

Here's a patch. It requires #8716 (but is independent of either #10450 or #10452). The fix was to make the various degeneracy matrix commands pass around an explicit target space, rather than just a target level, as the argument.

comment:3 Changed 9 years ago by davidloeffler

Depends on #8716

comment:4 Changed 8 years ago by davidloeffler

  • Description modified (diff)

comment:5 Changed 8 years ago by davidloeffler

  • Description modified (diff)

comment:6 follow-up: Changed 8 years ago by johanbosman

  • Reviewers set to Johan Bosman
  • Status changed from needs_review to positive_review

The code looks sound and is well documented. All long doctests pass. I played around with it and couldn't produce any false results. I did get the following while applying the patch:

applying trac_10453_gamma1_old_submodule.patch
patching file sage/modular/hecke/ambient_module.py
Hunk #10 succeeded at 883 with fuzz 1 (offset 6 lines).
now at: trac_10453_gamma1_old_submodule.patch

but according to this discussion, fuzz 1 is okay. Hence: positive review. ;).

comment:7 in reply to: ↑ 6 ; follow-up: Changed 8 years ago by leif

  • Description modified (diff)

Replying to johanbosman:

I did get the following while applying the patch:

applying trac_10453_gamma1_old_submodule.patch
patching file sage/modular/hecke/ambient_module.py
Hunk #10 succeeded at 883 with fuzz 1 (offset 6 lines).
now at: trac_10453_gamma1_old_submodule.patch

but according to this discussion, fuzz 1 is okay. Hence: positive review. ;).

In this case, you could re-export the patch and upload the rebased version.

IMHO one should inspect any fuzz anyway.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by davidloeffler

Replying to leif:

IMHO one should inspect any fuzz anyway.

I have inspected the fuzz, which is caused by patch #10664 having gone in since this was written. It is not a problem; the issue fixed by #10664 is completely orthogonal to this one.

comment:9 in reply to: ↑ 8 Changed 8 years ago by leif

Replying to davidloeffler:

I have inspected the fuzz, which is caused by patch #10664 having gone in since this was written. It is not a problem; the issue fixed by #10664 is completely orthogonal to this one.

Thanks, I hadn't [yet].

comment:10 Changed 8 years ago by leif

  • Merged in set to sage-4.7.2.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.