search

brick / math

Arbitrary-precision arithmetic library for PHP
MIT License
1.83k stars 78 forks source link

Rounding bug, or am I just dumb? #72

Closed jurchiks closed 2 years ago

jurchiks commented 2 years ago 
BigDecimal::of(0.02485333644)->toScale(4, RoundingMode::HALF_UP)->toFloat(), // 0.0249 <- correct
BigDecimal::of(0.02485333644)->toScale(3, RoundingMode::HALF_UP)->toFloat(), // 0.025 <- correct
BigDecimal::of(0.02485333644)->toScale(2, RoundingMode::HALF_UP)->toFloat(), // 0.02 <- WHAT???
BigDecimal::of(0.0248)->toScale(2, RoundingMode::HALF_UP)->toFloat(), // 0.02 <- WHY???
BigDecimal::of(0.025)->toScale(2, RoundingMode::HALF_UP)->toFloat(), // 0.03 <- CORRECT

What is going on here? Is it only looking at the 4 and rounding down instead of rounding 5, then 8, then 4(5)? This is not the behaviour I was expecting. I am using version 0.9.3, which has no code differences with 0.10.0, the latest version at the time of writing this, so it's not like I'm running an outdated, buggy version.

jurchiks commented 2 years ago

Update: it has come to my attention that apparently I have been taught wrong in school and I should only look at the digit before the decimal place to round to... Weird how I only now found out about this.

BenMorel commented 2 years ago

Hi, to round to 0.03 in rounding mode HALF_UP, your number should be >= 0.025, so the behaviour is the expected one here.

By the way, beware with floats, they’re converted to strings before being used, so you may experience rounding errors there too (unrelevant to your example, though).

jurchiks commented 2 years ago

Honestly, it feels like loss of precision to round this way, I really don't understand why we're only supposed to look at the previous digit. Unfortunately I'll now have to remember this and re-learn.

You mean rounding due to not enough digits? Yeah nah, I don't convert to float until the final value, which is always 3 decimal places or less.

BenMorel commented 2 years ago

I really don't understand why we're only supposed to look at the previous digit.

We’re not, I suggest you re-read the docs for HALF_UP and my comment above.