Opened 14 years ago

Closed 14 years ago

#2579 closed defect (fixed)

[with patch, positive review] Inconsistency in integer quotient

Reported by: rishi Owned by: somebody
Priority: major Milestone: sage-2.11
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mabshoff)

sage: a=-17
sage: a//4
-5
sage: a.div(4)
-4
sage: a.mod(4)
3

I recommend we redefine

def div(self, other):
    q,_ = self.quo_rem(other)
    return q

Attachments (1)

integerdiv.patch (1.3 KB) - added by rishi 14 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 14 years ago by rishi

sage: a=-17
sage: a4
-5
sage: a.div(4)
-4
sage: a.mod(4)
3

I recommend we redefine

def div(self, other):

q,_ = self.quo_rem(other)
return q

comment:2 Changed 14 years ago by mabshoff

  • Description modified (diff)

comment:3 Changed 14 years ago by cwitty

If we want a.div(b) to be floor(a/b) (which I agree we probably do, if we want the method to exist at all), the correct fix is to change from mpz_tdiv_qr to mpz_fdiv_q.

comment:4 in reply to: ↑ description Changed 14 years ago by rishi

I think the basis logic should be as below. Since this will make the remainder always positive.

if mpz_sgn(_other.value) == 1:
            mpz_fdiv_q(q.value, _self.value, _other.value)        
else:
            mpz_cdiv_q(q.value, _self.value, _other.value)

Changed 14 years ago by rishi

comment:5 Changed 14 years ago by rishi

I used quo_rem to redefine div. I would have essentially copied and pasted quo_rem otherwise.

comment:6 Changed 14 years ago by mhansen

  • Summary changed from Inconsistency in integer quotient to [with patch, positive review] Inconsistency in integer quotient

Looks good to me.

comment:7 Changed 14 years ago by mabshoff

  • Description modified (diff)

comment:8 Changed 14 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 2.11.alpha0

Note: See TracTickets for help on using tickets.