AdamWhiteHat / BigDecimal

An arbitrary-precision decimal (base 10) floating-point number class.
MIT License
47 stars 15 forks source link

BUG: value changed when creating a BigDecimal form double #49

Closed Bykiev closed 3 months ago

Bykiev commented 3 months ago

Description When creating a BigDecimal from double the value is changed

Error/Exception Message No

Expected Behavior The value should be changed

Actual Behavior The double value changes

Steps To Reproduce var res1 = new BigDecimal(1.7976931348623157);

The result is 1.7976931348623158, should be 1.7976931348623157

The BigDecimalParse(1.7976931348623157); returns correct result

AdamWhiteHat commented 3 months ago

Hi I just wanted to say I seen this and acknowledge it's a legitimate bug. I didn't want you to think I haven't seen it yet or was ignoring it.

Initially I though this was just System.Double that was doing the conversion before it ever got to BigDecimal to interpret, but no; the last digit flips when BigDecimal is converting the Double to a BigDecimal, specifically when it is multiplying it by a large power of ten. It probably has to do with the representation of the number in base 2 vs in base 10, or with however BigInteger stores its representation.

I was going to replace this multiply loop with parsing the string representation of the double, but I was hitting snags and edge cases with that too. I still haven't quite worked all of them out, and I begin to question the value of that approach.

I will try and resolve this over the weekend.

One thing to note here: As you are probably aware, there are some values of Double that do flip the last decimal place just by virtue of being instantiated as a Double. 1.7976931348623158 would be on such example. Although there still may be ways around this. The code (1.7976931348623158d).ToString("F30") for example returns the string "1.797693134862315744726402044762" so I guess it really has to do more with the way the value was being presented.

Protiguous commented 3 months ago

In the quick tests that I tried, using BigDecimal.Parse() on any Single or Double values (as strings) parsed and stored the BigDecimal correctly.

I know that parsing is going to be much slower initially, but I'd rather have accuracy than speed.

Protiguous commented 3 months ago

FYI, I am also working on some Span and Memory parsing for performance.

AdamWhiteHat commented 3 months ago

using BigDecimal.Parse() on any Single or Double values (as strings) parsed and stored the BigDecimal correctly.

Well not for me, it doesn't. But then again, I added a few more tests testing several edge cases, including the one from the start of this issue. If you pull the latest from the master branch, you should see a single commit, one that adds a few more tests to the file TestBigDecimalCritical.cs under the test project. Pull that down and rerun the tests and you should see what I mean.

If you happen to get all the tests passing after including these new one, please submit a pull request.

AdamWhiteHat commented 3 months ago

This should be fixed now.