Greedy algorithm odd behavior

Question

I've implemented the Greedy algorithm to solve Egyptian fractions, however I'm getting some unexpected results. Here's my code

from math import ceil
from fractions import Fraction

def go(frac):
    ret = []
    while frac > 0:
        if frac.numerator == 1:
            ret.append(frac)
            break
        x = Fraction(1, ceil(frac.denominator / frac.numerator))
        frac -= x
        ret.append(x)
    return ret

input1 = int(raw_input('numerator: '))
input2 = int(raw_input('denominator: '))

print go(Fraction(input1, input2))

I constantly am getting the error "TypeError: both arguments should be Rational instances"

I've been logging and it crashes upon the first iteration of the while loop.

EDIT: the error in detail is:

File "egypt.py", line 19, in <module>
print go(Fraction(input1, input2))
File "egypt.py", line 10, in go
x = Fraction(1,ceil(frac.denominator / frac.numerator))
File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/fractions.py", line 158, in __new__
raise TypeError("both arguments should be "
TypeError: both arguments should be Rational instances

Why is this? Thank you.

Erik Kaplun · Answer 1 · 2013-10-01T13:50:47.610

2

You have 2 problems in your code.

you're dividing int with int which always returns an int; in your case, you're dividing a / b where a < b so it'll always be rounded down to 0.
Then, you ceil() that, which returns a float (0.0) which is something Fraction doesn't like;it wants ints.

So try this instead:

Fraction(1, int(ceil(float(frac.denominator) / frac.numerator)))

The rest of the code looks good.

edited Oct 01 '13 at 13:50

answered Oct 01 '13 at 13:45

Erik Kaplun

37,128
15
99
111

score 0 · Accepted Answer · answered Oct 01 '13 at 13:42

0

Try changing this:

x = Fraction(1, ceil(frac.denominator / frac.numerator))

to this:

x = Fraction(1,int(ceil(frac.denominator / float(frac.numerator))))

answered Oct 01 '13 at 13:42

mrip

14,913
4
40
58

1

First, `frac.denominator` and `frac.numerator` are ints, so the division is integer division, which rounds down and can result in 0 values, so you need to cast to a float to get the division right. Second, `ceil` returns a float, so you need to convert to an int because `Fraction` requires two ints. – mrip Oct 01 '13 at 13:47

Greedy algorithm odd behavior

2 Answers2