1

Be it the numbers 'a', 'b', and 'c':

a = 77033412951888085
b = a - 5
c = 2840988860479

A division produces the following result

>>> a/c, b/c
(27115.0, 27115.0)

Attempting to solve a CodeWars' Kata, I'm required to calculate a = 14949283383840498 * 27115 / 5262)

However, instead of getting

77033412951888085

I get

7.703341295188808e+16

... which is... only but slightly wrong, sigh. Yet it's wrong enough to prevent me from passing, as it's interpreted as 77033412951888080. And...

>>> 14949283383840498 * 27115 / 5262 == 77033412951888085
False

EDIT:

As @GreenCloakGuy 's comment solves my issue (as presented), I've decided to add actual code (and tests), since the given solution does not work for me. I'm using Python 3.8.6

import functools

def gcd(a:int, b:int) -> int:
    'Recursive function to return gcd of a and b'
    if a == 0:
        return b
    return gcd(b % a, a)

def lcm(a:int, b:int) -> int:
    'Function to return LCM of two numbers'
    return a / gcd(a, b) * b

def convertFracts(fractions:list) -> list:
    if len(fractions) == 0:
        return []
    denominators = [i[1] for i in fractions]
    LCM = functools.reduce(lambda a, b: lcm(a, b), denominators)
    # Here I attempt so use @GreenCloakGuy 's solution, and use floor division.
    return [[num * LCM // den, LCM] for num, den in fractions]

import unittest
class TestSolution(unittest.TestCase):
    #@unittest.skip
    def test_sample(self):
        a = [[1, 2], [1, 3], [1, 4]]
        b = [[6, 12], [4, 12], [3, 12]]
        self.assertEqual(convertFracts(a), b)

    @staticmethod
    def simplifyFractions(result: list):
        for numerator, denominator in result:
            GCD = gcd(numerator, denominator)
            yield [numerator/GCD, denominator/GCD]

    @unittest.expectedFailure
    def test_failed(self):
        #Test result I got problems with
        test_result = [[77033412951888085, 14949283383840498],
                       [117787497858828, 14949283383840498],
                       [2526695441399712, 14949283383840498]
                       ]

        #Infer initial simplified fractions
        starting_fractions = [fraction for fraction in self.simplifyFractions(test_result)]
        print('Infered starting fractions: ', starting_fractions)

        my_result = convertFracts(starting_fractions)
        print('My result: ', my_result)
        my_result_as_int = list(map(lambda x: [int(x[0]), int(x[1])], convertFracts(my_result)))
        print('My result as int: ', my_result_as_int)
        self.assertEqual(test_result, my_result_as_int)

eltbus
  • 435
  • 1
  • 7
  • 15
  • 4
    Use integer division `//` instead of true division `/`. Since this _actually should_ result in an integer, no precision is lost by removing a decimal point; but the result is large enough that the floating point standard can't represent it exactly, so it has to stay an integer the entire time (python integers can get arbitrarily large). – Green Cloak Guy Oct 18 '20 at 23:41
  • @GreenCloakGuy, thank you for your answer. It does solve my question (as I presented it), but unfortunately, it does not solve my issue. I'm going to edit my questing to include actual code. – eltbus Oct 19 '20 at 12:09
  • 1
    `77033412951888085` is a 57 bit value. I'd expect saving/displaying that as a FP incurs a round to the most significant 53 bits. If 57 bits needed, use types that support the needed precision, not the default FP one. – chux - Reinstate Monica Oct 19 '20 at 13:39
  • 1
    @eltbus: You need to use integer division *everywhere* in your code. In the code you posted, you still have `a / gcd(a, b) * b` (should be `a // gcd(a, b) * b`) and `yield [numerator/GCD, denominator/GCD]` (should be `yield [numerator//GCD, denominator//GCD]`). Incidentally, Python 3.9 totally takes the fun out of this: it has a `math.lcm` function that takes multiple arguments: `math.lcm(2, 6, 7, 8) -> 168`. – Mark Dickinson Oct 19 '20 at 19:56
  • EDIT: I'd previously written that you corrections didn't fix the issue. I'm just dumb as anything and was still calling the same function. With you corrections, it works flawlessly @MarkDickinson ! (and of course thank you, Green Cloak Guy, who first suggested the use of integer division, which I applied incorrectly). – eltbus Oct 20 '20 at 08:32

1 Answers1

0

Two ways to make it work:

  1. Correct use of integer division (thanks to @GreenCloakGuy and @MarkDickinson)
  2. Using decimal.Decimal. Could be a duplicate of this question

Decimal has worked for me. Here's the updated code (only imports and updated function):

With correct use of //

import functools

#def gcd

def lcm(a:int, b:int) -> int:
    'Function to return LCM of two numbers'
    return a // gcd(a, b) * b

def convertFracts(fractions:list) -> list:
    if len(fractions) == 0:
        return []
    denominators = [i[1] for i in fractions]
    least_common_multiple = functools.reduce(lambda a, b: lcm(a, b), denominators)
    return [[num * least_common_multiple // den, least_common_multiple] for num, den in fractions]

With Decimal:

import functools
from decimal import Decimal

#def gcd

def lcm(a:int, b:int) -> int:
    'Function to return LCM of two numbers'
    return a // gcd(a, b) * b

def convertFracts(fractions:list) -> list:
    if len(fractions) == 0:
        return []
    denominators = [i[1] for i in fractions]
    least_common_multiple = Decimal(functools.reduce(lambda a, b: lcm(a, b), denominators))
    return [[Decimal(num) * least_common_multiple / Decimal(den), least_common_multiple] for num, den in fractions]

Decimal would have also worked if lcm used float division, but I figured I'd make the fix anyways, as it makes sense.

eltbus
  • 435
  • 1
  • 7
  • 15