Opened 17 years ago

Closed 16 years ago

#569 closed Patches (Duplicate)

rational::operator<(rational) fails due to overflow

Reported by: dbenbenn Owned by: Jonathan Turkanis
Milestone: Component: None
Version: None Severity:
Keywords: Cc:

Description

Consider boost::rational<long>.  Pick two objects of
that type at random (that's well defined, since there
are only finitely many objects of that type), and pick
them to both have the same sign.  Call them a and b. 
Then the expression "a < b" returns the wrong value 50%
of the time!  The problem is that the calculation in
rational.hpp can overflow, and that overflow actually
happens most of the time.

Here's a patch that implements operator< without
overflow.  It's somewhat less efficient (I haven't
benchmarked it) but it gives the right answer.

Possibly the new version of operator< has bugs.  I've
run a few hundred million random tests, with the right
answer every time (I've used bc to check the answers),
but there might be a subtle problem.  In particular,
the new code assumes that integer division always
truncates towards 0, even when dividing negative
integers.  Perhaps that is not true on some obscure
platforms/compilers?

Change History (2)

comment:1 by dbenbenn, 17 years ago

Logged In: YES 
user_id=95581

Here's a new patch that uses std::numeric_limits to select
the more efficient, direct comparison method for unbounded
types, and only uses the overflow-avoiding method for types
that can overflow.

comment:2 by Daryle Walker, 16 years ago

Status: assignedclosed
Logged In: YES 
user_id=551024

The problem was also described as bug #798357.
Your issue should be fixed since I've fixed the other
bug. I used continued-fraction expansion, with the
components found by the Euclidean GCD algorithm
and forcibly floor-rounding division & modulus.

Note: See TracTickets for help on using tickets.