Opened 11 years ago

Closed 11 years ago

#9955 closed defect (fixed)

Rational(3)%Rational(-1) fails

Reported by: schilly Owned by: AlexGhitza
Priority: major Milestone: sage-4.6.2
Component: basic arithmetic Keywords:
Cc: Merged in: sage-4.6.2.alpha4
Authors: André Apitzsch Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

This is inconsistent 

sage: Rational(3)%Rational(-1)
ZeroDivisionError: Inverse does not exist.

but 

sage: 3%(-1)
0

Attachments (1)

trac_9955.patch​ (1.0 KB) - added by aapitzsch 11 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 years ago by cremona

This is caused by the following simpler bug: 

sage: a=Integer(3)
sage: b=Integer(-1)
sage: a.inverse_mod(b)
---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)
...

ZeroDivisionError: Inverse does not exist.

which is easy to fix. In sage.rings.integer.Integer.inverse_mod there is special case for modulus n=1 but not for -1. Either ass this special case, or replace n by abs(n).

comment:2 Changed 11 years ago by aapitzsch

  • Status changed from new to needs_review

comment:3 Changed 11 years ago by cremona

  • Status changed from needs_review to needs_work

The patch looks right and I tested that it works (but did not yet test the whole library).

BUT you need to add a doctest to show that the bug has been fixed. There's a similar doctest in the same function, so just add something similar.

Then I'll look at it again.

Changed 11 years ago by aapitzsch

comment:4 follow-up: Changed 11 years ago by aapitzsch

  • Authors set to André Apitzsch
  • Status changed from needs_work to needs_review

doctest added

comment:5 in reply to: ↑ 4 Changed 11 years ago by cremona

  • Reviewers set to John Cremona
  • Status changed from needs_review to positive_review

Replying to aapitzsch:

doctest added

Thanks! Positive review.

comment:6 Changed 11 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.