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

precision over 1000 causing DecimalError for trig functions on a BigNumber #1385

Open
husayt opened this issue Jan 17, 2019 · 14 comments
Open

Comments

@husayt
Copy link

husayt commented Jan 17, 2019

When I try big precisions over 1000 this is the error I get:

: Error. [DecimalError] Precision limit exceededA

If there is a limit to precision, would be nice to document this, otherwise this is an issue

@josdejong
Copy link
Owner

josdejong commented Jan 17, 2019

According to the docs of Decimal.js:

The number of digits of value is not limited, except by JavaScript's maximum array size and, in practice, the processing time required.

Looking in the code of decimal.js though, I see that pi is a hardcoded with 1025 digits (LN10 has the same), and there is an error thrown if you request pi with a higher precision than that:

https://github.com/MikeMcl/decimal.js/blob/master/decimal.js#L33
https://github.com/MikeMcl/decimal.js/blob/master/decimal.js#L3158-L3161

To resolve this I guess we could fallback on calculating pi on the fly with higher precision.

@josdejong
Copy link
Owner

Note that calculations without pi can be done with higher precision without any issue:

math.config({ precision: 10000 })

math.bignumber(1).div(3).toString() // 0.3333333... many digits...

@josdejong josdejong changed the title precision over 1000 causing DecimalError precision over 1000 causing DecimalError when using pi Jan 17, 2019
@husayt
Copy link
Author

husayt commented Jan 17, 2019 via email

@harrysarson
Copy link
Collaborator

Could you share an example of code that throws that error?

@husayt
Copy link
Author

husayt commented Jan 18, 2019

Yes sure:
https://codesandbox.io/s/9240x1xqzp

Actually, seems problem is not with eval, but with math.format

@josdejong
Copy link
Owner

That's interesting. Looks indeed that math.format triggers the limit on 1025 that Decimal.js has. Would be great if anyone can look into the cause and possible workaround for this.

On a side note: I think working with a precision over 1000 digits is theoretical? Or is there a real daily use case for this?

@husayt
Copy link
Author

husayt commented Jan 20, 2019 via email

@ericman314
Copy link
Collaborator

I'm taking a look into it right now.

@ericman314 ericman314 changed the title precision over 1000 causing DecimalError when using pi precision over 1000 causing DecimalError when formatting a BigNumber Jan 21, 2019
@ericman314
Copy link
Collaborator

It fails because the auto config setting causes the formatter to calculate the base 10 logarithm of the bignumber. It compares it to upperExp and lowerExp to see whether it should format in scientific notation or not:

      const exp = value.abs().logarithm()
      if (exp.gte(lowerExp) && exp.lt(upperExp)) {
        // normal number notation
      } else {
        // exponential notation
      }

But value has a property, e, which is equivalent to floor(log10(value)), so replacing the log calculation with the property e works as long as upperExp and lowerExp are integers:

      const exp = value.e
      if (exp >= lowerExp && exp < upperExp) {
        // normal number notation
      } else {
        // exponential notation
      }

I would presume that normally, lowerExp and upperExp would be integers. At least this is the case in all of the unit tests, which continue to pass.

#1387 is a fix that works so long as lowerExp and upperExp are integers.

@husayt
Copy link
Author

husayt commented Jan 21, 2019

@ericman314 Thanks for a solution and detailed explanation.

@husayt husayt closed this as completed Jan 21, 2019
@josdejong
Copy link
Owner

@husayt is the fix of Eric enough to solve your issue? It means that precision above 1000 digits works for arithmetic operations like multiplication and addition, but still not for trigonometric operations involving pi.

@husayt
Copy link
Author

husayt commented Jan 21, 2019 via email

@josdejong
Copy link
Owner

👍 then I will keep this issue open as a reminder

@josdejong josdejong reopened this Jan 21, 2019
@josdejong
Copy link
Owner

The issue with math.format is now fixed in v5.4.2 thanks to Eric's fix.

@harrysarson harrysarson changed the title precision over 1000 causing DecimalError when formatting a BigNumber precision over 1000 causing DecimalError for trig functions on a BigNumber Apr 12, 2019
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

4 participants