Opened 11 years ago

Closed 10 years ago

#10452 closed defect (fixed)

Weird behaviour with cusps

Reported by: davidloeffler Owned by: craigcitro
Priority: trivial Milestone: sage-4.6.2
Component: modular forms Keywords: cusps
Cc: Merged in: sage-4.6.2.alpha4
Authors: David Loeffler Reviewers: Alex Ghitza
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The command "reduce_cusp" seems to be a bit flaky -- here are two perfectly standard inputs that cause it to fail in different ways:

sage: G = Gamma1(5); G
Congruence Subgroup Gamma1(5)
sage: G.reduce_cusp(1/2) # works
1/2
sage: G.reduce_cusp(3) # boom (1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/masiao/<ipython console> in <module>()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/modular/arithgroup/congroup_gammaH.pyc in reduce_cusp(self, c)
    658         """
    659 
--> 660         return self._reduce_cusp(c)[0]
    661 
    662     def _reduce_cusp(self, c):

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/modular/arithgroup/congroup_gammaH.pyc in _reduce_cusp(self, c)
    725         if d == 1:
    726             if v in H:
--> 727                 return Cusps((0,1)), 1
    728             if (N-v) in H:
    729                 return Cusps((0,1)), -1

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6462)()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3118)()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3021)()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/rings/integer.so in sage.rings.integer.Integer.__init__ (sage/rings/integer.c:7312)()

TypeError: unable to coerce <type 'tuple'> to an integer
sage: G.reduce_cusp(oo) # boom (2)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/home/masiao/<ipython console> in <module>()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/modular/arithgroup/congroup_gammaH.pyc in reduce_cusp(self, c)
    658         """
    659 
--> 660         return self._reduce_cusp(c)[0]
    661 
    662     def _reduce_cusp(self, c):

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/modular/arithgroup/congroup_gammaH.pyc in _reduce_cusp(self, c)
    695         N = int(self.level())
    696         Cusps = c.parent()
--> 697         v = int(c.denominator() % N)
    698         H = self._list_of_elements_in_H()
    699 

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/element.so in sage.structure.element.Element.__getattr__ (sage/structure/element.c:2666)()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.getattr_from_other_class (sage/structure/parent.c:2840)()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.raise_attribute_error (sage/structure/parent.c:2638)()

AttributeError: 'PlusInfinity' object has no attribute 'denominator'

In each case G.reduce_cusp(Cusp(...)) gives the right answer, so the problem is apparently that various code expects the input to be a Cusp object and raises problems when it isn't.

Attachments (1)

trac_10452-cusps.patch (12.9 KB) - added by davidloeffler 11 years ago.
Apply over "trac_8716-gamma_h_modforms.patch" from #8716

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 years ago by davidloeffler

  • Status changed from new to needs_review

Here's a patch. As expected, the problem was that input wasn't being correctly converted to type Cusp. I've also removed a pointless duplicate method stub "are_equivalent", and moved the Sturm bound code to a more natural place.

Changed 11 years ago by davidloeffler

Apply over "trac_8716-gamma_h_modforms.patch" from #8716

comment:2 Changed 11 years ago by davidloeffler

I just noticed that my original patch conflicted with #8716, which has a positive review, so I've uploaded a new rebased version.

comment:3 Changed 11 years ago by davidloeffler

  • Authors set to David Loeffler

comment:4 Changed 11 years ago by davidloeffler

  • Priority changed from major to trivial

comment:5 Changed 10 years ago by AlexGhitza

  • Reviewers set to Alex Ghitza
  • Status changed from needs_review to positive_review

Looks good to me.

comment:6 Changed 10 years ago by jdemeyer

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