Opened 13 years ago

Last modified 10 years ago

#5048 closed enhancement

congruence subgroups are not integrated into the coercion model — at Version 15

Reported by: ncalexan Owned by: craigcitro
Priority: major Milestone: sage-5.0
Component: modular forms Keywords: congruence subgroup coercion, sd35
Cc: GeorgSWeber Merged in:
Authors: David Loeffler Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by davidloeffler)

sage: Gamma0(10).1 * Gamma0(5).2
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/ncalexan/.sage/temp/sage.math.washington.edu/4030/_home_ncalexan__sage_init_sage_0.py in <module>()
----> 1 
      2 
      3 
      4 
      5 

/scratch/nca/sage-3.3.alpha0-sage.math-only-x86_64-Linux/local/lib/python2.5/site-packages/sage/structure/element.so in sage.structure.element.MonoidElement.__mul__ (sage/structure/element.c:7375)()
    849 
    850 
--> 851 
    852 
    853 

TypeError: unsupported operand parent(s) for '*': 'Congruence Subgroup Gamma0(10)' and 'Congruence Subgroup Gamma0(5)'

Apply *either*

(There is no difference in functionality either way, it's just that the original patch won't apply because some context lines changed.)

Change History (18)

comment:1 Changed 13 years ago by AlexGhitza

  • Type changed from defect to enhancement

comment:2 Changed 12 years ago by davidloeffler

  • Component changed from number theory to modular forms
  • Owner changed from was to craigcitro

Changed 11 years ago by davidloeffler

patch against 4.6.1.alpha3

comment:3 Changed 11 years ago by davidloeffler

  • Authors set to David Loeffler
  • Report Upstream set to N/A
  • Status changed from new to needs_review

Here's a patch. I thought the simplest solution was to arrange that the parent of all congruence subgroup elements was always the (globally unique) SL2Z object, i.e. to make the various subgroup classes "facade parents" (like the prime integers example in the category docs).

Changed 11 years ago by davidloeffler

Version that will apply happily after #10452

comment:4 Changed 11 years ago by davidloeffler

I just realised that the patch I just uploaded conflicts, in a rather trivial way, with the patch at #10452. The second patch will apply on top of the patch at #10452. The two patches are identical other than one line of diff context.

comment:5 Changed 11 years ago by davidloeffler

Oh dear, it looks like the trac buildbot is trying to apply both patches at once, hence the red light! But it seems to have done one successful run with only the first patch installed.

comment:6 Changed 11 years ago by davidloeffler

Apply trac_5048-sl2z_coercion-rebased_for_10452.patch Depends on #10452

comment:7 Changed 11 years ago by davidloeffler

  • Priority changed from major to minor

Changed 10 years ago by davidloeffler

version rebased to 4.7.1.alpha4 + #11422

comment:8 Changed 10 years ago by davidloeffler

I've uploaded a version that is rebased to apply happily over the positively-reviewed patch #11422. This is intended to form part of a series #10335 - #11422 - #11598 - this ticket - #10453 -

#11601; but the patch itself is independent of #11598 and of the tickets later in the series.

comment:9 Changed 10 years ago by davidloeffler

  • Priority changed from minor to major

I'm raising the priority of this to "major" since it is a prerequisite for the (much more significant) patch #11601.

comment:10 Changed 10 years ago by GeorgSWeber

  • Cc GeorgSWeber added

comment:11 Changed 10 years ago by GeorgSWeber

Hmmm, is there a quick and easy explanation for the change in the last line of the patch (file "sage/modular/modform/constructor.py")?

  • 87 <class 'sage.modular.arithgroup.congroup_gamma0.Gamma0_class'>,

+ 87 <class 'sage.modular.arithgroup.congroup_gamma0.Gamma0_class_with_category'>,

I mean, this might be necessary, but due to the rest of this patch or due to some earlier patch??

comment:12 Changed 10 years ago by davidloeffler

It is necessary because the arithmetic subgroup init routine now calls Group.__init__ , which was not previously the case. The group init routine subsequently calls various other init routines that link arithmetic subgroups into Sage's category framework, with all of its baroque subtleties involving dynamic classes etc.

comment:13 Changed 10 years ago by davidloeffler

  • Description modified (diff)

comment:14 Changed 10 years ago by davidloeffler

  • Description modified (diff)

Georg: did you get any chance to look at this again? Since #11422 is still waiting around, I've clarified the description to point out that both the pre-#11422 and post-#11422 patches exist and are equally valid.

comment:15 Changed 10 years ago by davidloeffler

  • Description modified (diff)
Note: See TracTickets for help on using tickets.