commanderx16 / x16-rom

Other
153 stars 43 forks source link

LOG(2) inaccurate (slightly) #363

Open Jaxartes opened 1 year ago

Jaxartes commented 1 year ago

The result of LOG(2) in BASIC is a little bit off. It's debatable whether this is significant, since floating point numbers are generally imprecise. The result looks right when printed, but as seen in memory it's a little bit different from what the Commodore 64 calculates and from what it should be.

On both, "PRINT LOG(2)" shows .693147181. There are a few ways to see it's different:

Seen with current ROM code (5d8ba9f36b4c4fb4) and also on rather older ROM (b6a0f0c3daca1ca3). Seen with LOG(2) and LOG(2+2↑-30) and nothing else I've tried, including LOG(2.000001).

MJoergen commented 1 year ago

Interesting. I did change the ADD, MULT, and SQR routines in the math library to more optimized versions (see commit 005c504) two years ago. It could be worth looking into, whether this change is the source of the error.

I don't have the opportunity to test this myself at the moment, so this is just informational, in case someone else picks up from here.

Jaxartes commented 1 year ago

Confirmed in testing that with commit 005c504124c64a81, the result of "PRINT 1000*(.7-LOG(2))" changed from 6.85281931 (as on C64) to 6.85281919 (as now).

Jaxartes commented 1 year ago

Strangely, it appears 'fmult' is more accurate than before commit 005c504 (at least in one relevant case) but something in 'log' that didn't change was wrong. Or else there's something strange about how 'facov' (the overflow/rounding byte) is handled.

In both the current code and before 005c504, LOG(2) ends with multiplying 'log2' by the following value in 'fac': 81 80 00 00 00 80 (where the last byte I've listed is that in 'facov' though it's not contiguous in memory). The result given for that is different, 80 B1 72 17 F8 2C with the old code and 80 B1 72 17 F8 B0 with the new.

Assuming 'facov' is a straightforward 8 bit extension of the mantissa of FAC then the new result is more accurate. Is that assumption right? I don't know.

Data: 80 31 72 17 f8 .. = 0.6931471806019545 (log2 in memory) 81 80 00 00 00 80 = 1.0000000002328306 (fac passed to fmult) 80 B1 72 17 F8 2C = 0.6931471806419722 (old code, better LOG(2)) 80 B1 72 17 F8 B0 = 0.6931471807620255 (new code, better product)

MJoergen commented 1 year ago

I seem to remember that 'facov' indeed is just the eight following bits of the accumulator, used for extra accuracy. Or maybe 'facov' really only contains one or two bits (rather than eight), I'm not sure.

Now, the line "fac passed to fmult" should contain the exact real value 1.0, since this should be the log base 2 of the input value. I believe the value shown is the result of a lenghty calculation, so presumably there is some accumulated rounding error in this preceding calculation.