AndriSignorell/DescTools

LCM integer overflow

NicChr opened this issue · 7 comments

Hello, I noticed that LCM() is silently producing integer overflows.
Please see the below reprex.

library(DescTools)

packageVersion("DescTools")
#> [1] '0.99.50.3'

lcm_1_22 <- DescTools::LCM(1:22)
lcm_1_22
#> [1] 232792560

cat("Result:", DescTools::LCM(lcm_1_22, 23L))
#> Result: 1059261584
cat("Expected:", (lcm_1_22 / DescTools::GCD(lcm_1_22, 23)) * 23)
#> Expected: 5354228880

Created on 2023-11-18 with reprex v2.0.2

@NicChr: Thanx for this hint. You are exceeding the integer value range of 2^31. With this in mind, I looked for and found more modern alternatives. C++ version 17 allows calculation with long integers. This only postpones the problem, but for our generation it should be enough ... ;-)
Please check and close the task, if ok.

@AndriSignorell I think this change may have broken the most recent release of this package for R versions >=4.0.0 and <4.3.0. Like you said the gcd and lcm functions were introduced in C++17, however R versions prior to 4.3.0 still default to C++14

Thanks for this hint @jtlevine. I wasn't aware of this, so I will upgrade the requirements to R >= 4.3.0 and resubmit. Time to move on anyway.... ;-)
The new version is by the way also 20% faster than the boost implementation.

Fixed:

packageVersion("DescTools")
#> [1] '0.99.52.1'
 
lcm_1_22 <- DescTools::LCM(1:22)
lcm_1_22
[1] 232792560
#> [1] 232792560

cat("Result:", DescTools::LCM(lcm_1_22, 23L))
#> Result: 5354228880> 
cat("Expected:", (lcm_1_22 / DescTools::GCD(lcm_1_22, 23)) * 23)
#> Expected: 5354228880

@NicChr: would you mind checking this and close the issue, if ok?

Thanks @AndriSignorell, looks good now!

Sorry to re-open the issue, for people still using R version smaller than 4.3.0, is there a general instruction on how to install the older compatible version of DescTools?