Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with Cusp constructor #3800

Closed
craigcitro opened this issue Aug 10, 2008 · 3 comments
Closed

Problem with Cusp constructor #3800

craigcitro opened this issue Aug 10, 2008 · 3 comments

Comments

@craigcitro
Copy link
Member

There are several issues with the Cusp.__init__ method. Here's an example:

sage: Cusp(1/3,0)
---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)

/Users/craigcitro/<ipython console> in <module>()

/sage/local/lib/python2.5/site-packages/sage/modular/cusps.py in __init__(self, a, b, construct, parent)
    194 
    195         elif isinstance(a, Rational):
--> 196             a = a/b
    197             self.__a = a.numer()
    198             self.__b = a.denom()

/Users/craigcitro/element.pyx in sage.structure.element.RingElement.__div__ (sage/structure/element.c:9074)()

/Users/craigcitro/coerce.pyx in sage.structure.coerce.CoercionModel_cache_maps.bin_op_c (sage/structure/coerce.c:5100)()

/Users/craigcitro/element.pyx in sage.structure.element.RingElement.__div__ (sage/structure/element.c:9057)()

/Users/craigcitro/coerce.pxi in sage.structure.element._div_c (sage/structure/element.c:16420)()

/Users/craigcitro/rational.pyx in sage.rings.rational.Rational._div_c_impl (sage/rings/rational.c:8135)()

ZeroDivisionError: Rational division by zero

This is relevant, since some computations with modular symbols on GammaH can lead to exactly this problem.

The attached patch rewrites the __init__ method, slightly improving the speed, and vastly improving the consistency. In particular, it was possible to construct two different cusps which both claimed to be Infinity (namely (1,0) and (-1,0)).

In the process of doing this, I exposed several bits of code that were taking advantage of this "loophole," which then needed to be fixed. In an attempt to make things more clear, I ended up doctesting all of sage/modular/modsym/boundary.py, so that's also included.

Component: modular forms

Issue created by migration from https://trac.sagemath.org/ticket/3800

@craigcitro craigcitro added this to the sage-3.1 milestone Aug 10, 2008
@craigcitro craigcitro self-assigned this Aug 10, 2008
@craigcitro
Copy link
Member Author

Attachment: trac-3800.patch.gz

@williamstein
Copy link
Contributor

comment:2

Attachment: sage-3800-referee_touch_up.patch.gz

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Aug 11, 2008

comment:3

Merged both patches in Sage 3.1.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants