Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple multiplication gets wrong answer #403

Open
ksaj opened this issue Jun 20, 2021 · 6 comments
Open

Simple multiplication gets wrong answer #403

ksaj opened this issue Jun 20, 2021 · 6 comments

Comments

@ksaj
Copy link

ksaj commented Jun 20, 2021

(* 123456789 123456789)
15241578750190520

The answer should be 15241578750190521.

@ksaj
Copy link
Author

ksaj commented Jun 20, 2021

Here's another example:

CL-USER> (* 99999995 99999995)
9999999000000024

The last digit should be a 5.

@davazp
Copy link
Member

davazp commented Jun 20, 2021

JSCL represents all numbers as Javascript "number" (floats). There is no distinction between floats and integer, and no bignums.

This was a more unavoidable limitation when we started JSCL. Nowadays it would be feasible to use Javascript bigints when possible to differentiate them from numbers.

But it does require a bit of work on the compiler! It sounds like a fun project for sure.

An open question is if using bigint would have bad performance implications or if the engines optimize for small bigints :-)

We could make it optional by (at least temporarily) using some constructors for numbers with something like internals.n(25).

@ksaj
Copy link
Author

ksaj commented Jun 20, 2021

I'm not a JS coder, so I was unaware of this limitation. The math should be coded instead of using "default" JS constructs once the numbers reach that tipping point, I suppose. It probably means there are a lot more examples like this, and probably examples that are off by more than one digit.

@davazp
Copy link
Member

davazp commented Jun 21, 2021

Indeed. Luckily there is a way foward now at least!

Until then though, anything that goes out of this range will be wrong:

Number.MAX_SAFE_INTEGER // => 9007199254740991

and functions like integerp and floatp will give incorrect results on integers. (floatp 1.0) ; => NIL and (integerp 1.0) ; => T.

It's surprising how lot of code does still work considering those issues 🙂

@vlad-km
Copy link
Member

vlad-km commented Jun 21, 2021

It's surprising how lot of code does still work considering those issues 🙂

Ugu;) Artifacts:

CL-USER> (floatp 1.0)
NIL
CL-USER> (floatp 1.00000000001)
T
CL-USER> (floatp 1.000000000001)
T
CL-USER> (floatp 1.000000000000001)
T
CL-USER> (floatp 1.0000000000000001)
NIL

sbcl

CL-USER> (1+  (expt 2 53))
9007199254740993
CL-USER> (expt 2 53)
9007199254740992

jscl

CL-USER> (1+  (expt 2 53))
9007199254740992
CL-USER> (expt 2 53)
9007199254740992

@davazp
Copy link
Member

davazp commented Jun 22, 2021

I opened a WIP pull request to see what bignum support might look like at #404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants