Skip to content

Implemented math.factorial(n) for calculating very large factorials with speed and accuracy. #2557

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

Closed
wants to merge 8 commits into from

Conversation

kmr-srbh
Copy link
Contributor

@kmr-srbh kmr-srbh commented Feb 27, 2024

Overview

  • The current implementation of math.factorial() is naive.
  • Returns 0 for factorial for large values due to integer overflow.

Screenshot from 2024-02-27 23-12-57

Solution

The industry standard for calculating factorial of a large number is complicated. It deals with Prime Number generation and Swing Numbers. These methods require BigInt for storing the large calculated value.

One fast method for going about calculating the calculation is to use an array to store the digits of the number as strings, in reverse, and do digit by digit multiplication. This method is used for implementing the algorithm.

I support the implementation of the Swing Number algorithms if one can understand and implement it correctly. The funny thing is that a Python implementation is available online.

Improvement

  • Works for large factorials
    Screenshot from 2024-02-27 23-17-56
  • Works for very large factorials
    Screenshot from 2024-02-27 23-19-13
  • Accurate
    Screenshot from 2024-02-27 23-22-05

Integration tests have not been added due to the prerequisite below.

Prerequisite

  • While the function is complete, it depends on String to int conversion in ASR #2554 for getting merged. Currently it just prints the output to stdout and returns 0.
  • The long assignment of digits is because LPython currently does not support list assignment like: my_list[0:4] = [1, 2, 3]. The assignments can be improved through the introduction of the above list assignment.

@kmr-srbh kmr-srbh closed this Feb 27, 2024
@kmr-srbh kmr-srbh deleted the fix-factorial branch February 27, 2024 18:39
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

Successfully merging this pull request may close these issues.

1 participant