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

Error when factoring. #1

Open
malisper opened this issue Mar 6, 2015 · 3 comments
Open

Error when factoring. #1

malisper opened this issue Mar 6, 2015 · 3 comments

Comments

@malisper
Copy link

malisper commented Mar 6, 2015

When executing (factor 327146098439579), I get the following error:

Index 2 out of bounds for (SIMPLE-ARRAY FIXNUM (2 46)), should be nonnegative and <2.
[Condition of type SB-INT:INVALID-ARRAY-INDEX-ERROR]

With the following backtrace:

0: (ULIMY-HMPQS::B-GENERATOR 2717 (5 4 3) 3)
1: (ULIMY-HMPQS::MAKE-CUBLETS (4 3) 1.3063123 5 45 3)
2: (ULIMY-HMPQS::MAKE-CUBLETS (3) 2.4202557 4 45 2)
3: (ULIMY-HMPQS::MAKE-CUBLETS NIL 3.4616485 3 45 1)
4: (ULIMY-HMPQS::A-GENERATOR)
5: (ULIMY-HMPQS::QS 327146098439579 :M 0 :B 0 :DELTA-T 0 :OMIT-BELOW 25 :SLP-LOG2 16 :ALFA 32 :REPORT2 NIL :RETURN2 ULIMY-HMPQS::FACTORS)
6: (ULIMY-HMPQS:HMPQS 327146098439579 :REPORT2 NIL)
7: (FACTOR 327146098439579)

I am running sbcl 1.2.5.

@smithzvk
Copy link
Owner

smithzvk commented Mar 6, 2015

Thanks for the report, I see it too. I'm looking into how to fix it but here are a few initial thoughts:

  1. The method used here, which is the default method in the library, was not written by me, and is not understood by me. I am maintaining it for another author that didn't make it available via ASDF/Quicklisp and doesn't appear to be active in the community anymore. I will attempt to figure out how to fix this while not breaking it (of course it is already broken) but as you can see, it seems to work for many other values so it will be difficult to verify this method. I'll try to find a way to do so reliably.
  2. There are other algorithms that are available in the library. These other methods are much simpler and fool proof, though often (but not always) slower. For instance, (cl-factoring-algorithms:brents-cycle 327146098439579) works fine and may be a valid work-around while I look into this. I may switch to using these simpler methods by default if the HMPQS method turns out to be unreliable. Slower is better than errors.

smithzvk added a commit that referenced this issue Mar 9, 2015
This is a work-around for issue #1 on the Github project page, that the
algorithm has an error rather than factoring the number 327146098439579.
While I gain the knowledge necessary to fix bug in HMPQS, I am switching
the factoring method to a simpler method.

After checking what effect this will have, I've found that it is
significantly slower on average than the HMPQS method, even in number
ranges where it should win.  That isn't too surprising as almost zero
effort has been put into optimizing it.
@malisper
Copy link
Author

I just realized you could use the default algorithm and switch to the other one when an error is thrown. It isn't pretty, but it is more efficient than just using the slower algorithm.

@smithzvk
Copy link
Owner

I haven't found time to look for a fix yet, in the mean time I changed the
default to a safer method and updated master. This is the way I'll keep it
until I can trust hmpqs. You can still access hmpqs via the algorithms
interface, but you will have to modify your hack to accommodate this change
once quicklisp picks up the change.

I'll keep this bug report open until I actually fix it.
On Mar 18, 2015 9:48 PM, "Michael Malis" notifications@github.com wrote:

I just realized you could use the default algorithm and switch to the
other one when an error is thrown. It isn't pretty, but it is more
efficient than just using the slower algorithm.


Reply to this email directly or view it on GitHub
#1 (comment).

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

2 participants