Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#10484 closed defect (fixed)

Chinese remainder code raises an error when called with Python ints

Reported by: davidloeffler Owned by: was
Priority: major Milestone: sage-4.6.2
Component: number theory Keywords: CRT
Cc: Merged in: sage-4.6.2.alpha0
Authors: David Loeffler Reviewers: Robert Bradshaw, Mike Hansen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Lots of the congruence subgroup code works intensively with small integers, and represents these as Python ints rather than Sage integers for speed reasons. This brought to light the following irritating bug:

sage: crt(int(2), int(3), int(5), int(7))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/storage/masiao/sage-4.6.1.alpha3/devel/sage-reviewing/<ipython console> in <module>()

/usr/local/sage/sage-4.6/local/lib/python2.6/site-packages/sage/rings/arith.pyc in crt(a, b, m, n)
   2737         return CRT_list(a, b)
   2738     g, alpha, beta = XGCD(m, n)
-> 2739     q, r = (b - a).quo_rem(g)
   2740     if r != 0:
   2741         raise ValueError("No solution to crt problem since gcd(%s,%s) does not divide %s-%s" % (m, n, a, b))

AttributeError: 'int' object has no attribute 'quo_rem'

Attachments (1)

trac_10484-crt_bug.2.patch (950 bytes) - added by davidloeffler 8 years ago.
new version

Download all attachments as: .zip

Change History (8)

comment:1 Changed 8 years ago by davidloeffler

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

Here's a tiny little patch (one line, one comment, one doctest).

comment:2 Changed 8 years ago by robertwb

  • Status changed from needs_review to positive_review

comment:3 Changed 8 years ago by mhansen

  • Reviewers set to Robert Bradshaw, Mike Hansen
  • Status changed from positive_review to needs_info

Shouldn't you use Integer(a) instead of ZZ(a) since it's about 3.5 times faster (on boxen)?

Changed 8 years ago by davidloeffler

new version

comment:4 Changed 8 years ago by davidloeffler

  • Status changed from needs_info to needs_review

Here's a new patch using Integer. Sorry, I meant to overwrite the original patch -- apply only the second one.

comment:5 Changed 8 years ago by mhansen

  • Status changed from needs_review to positive_review

Thanks!

comment:6 Changed 8 years ago by jdemeyer

  • Merged in set to sage-4.6.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:7 Changed 8 years ago by jdemeyer

  • Reviewers changed from Robert Bradshaw, Mike Hansen to Robert Bradshaw, Mike Hansen
Note: See TracTickets for help on using tickets.