primesearch / Mlucas

Ⓜ️ Ernst Mayer's Mlucas and Mfactor programs for GIMPS
https://mersenneforum.org/mayer/README.html
GNU General Public License v3.0
8 stars 2 forks source link

Fermat number-related patches to Mlucas.c #11

Closed xanthe-cat closed 8 months ago

xanthe-cat commented 8 months ago

Set of Fermat number related bug-fixes worked out between TD and CXC. Fixes:

More information about each of these issues is available at the links above.

xanthe-cat commented 8 months ago

Sorry for pulling out one of the bug-fixes in this pull at the last minute, however I need to do more testing of the GEC with residue shifting, and the minimum Fermat with a suitable exponent for this is F20. If it doesn’t cause a problem I’m happy to try another PR later. Edited to add: I’ve found a definite failure with non-zero residue shift and GEC (with all other variables normal), so this “fix” is now a non-starter.

tdulcet commented 8 months ago

Sorry for pulling out one of the bug-fixes in this pull at the last minute

No worries, please feel free to take as much time as you need to test everything. Thanks for adding those comments. I can merge this PR whenever you are ready.

I’ve found a definite failure with non-zero residue shift and GEC (with all other variables normal), so this “fix” is now a non-starter.

The zero shift is required for the largest exponents above the 256M FFT length. However, that exit(1) call is the most problematic here, as it causes Mlucas to silently exit when attempting to restart a Pépin test for any reason, even with the zero shift.

xanthe-cat commented 8 months ago

Yes, I think the exit(1) call is overkill. (I almost forgot to remove it.) I’m wondering, Teal; was the save file that you were trying to restart from using zero shift? (The GIMPS_status script will indicate whether it is or not.) I’ve reinstated that bit of code because, even for small exponents, there is no good reason not to use GEC in production mode; a different roadblock appears if one attempts using non-zero residue shift on F20 or greater, as the GEC tries to check the computation every million iterations by default, and even if Mlucas evaluates the correct residue at that point GEC can’t check that it is correct. It is possible to run the primality test with non-zero residue shift on the very small exponents where you only need to evaluate about half a million squarings for say F19, but for multi-million iterations in production mode it doesn’t seem wise or desirable to disable error checking. I’ve also tweaked the code slightly so it won’t exit if the worktodo item is not a Pépin test.

tdulcet commented 8 months ago

Thanks for the explanation. I did not realize that using a nonzero shift was implicitly disabling GEC. I definitely agree that users should be using GEC in production, especially on large exponents.

I’m wondering, Teal; was the save file that you were trying to restart from using zero shift?

Yes, the savefile was using a zero shift, but that exit call caused it to silently exit. Anyway, your latest changes look good to me.

tdulcet commented 8 months ago

No worries now as I just manually fixed it, but in the future when merging PRs, please use the "Squash and Merge" option. This will automatically squash all of the commits in the PR to a single commit before merging it, so that it does not add a bunch of unnecessary commits to the main branch.

xanthe-cat commented 8 months ago

Apologies, I did not see that that was both an available, and the preferred, option.