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

"Extra strong" Lucas pseudoprime #18946

Merged
merged 8 commits into from
Mar 27, 2020
Merged

"Extra strong" Lucas pseudoprime #18946

merged 8 commits into from
Mar 27, 2020

Conversation

Arpan612
Copy link
Contributor

@Arpan612 Arpan612 commented Mar 24, 2020

"Extra strong" Lucas pseudoprime 2nd condition corrected

References to other Issues or PRs

Fixes #18921

Brief description of what is fixed or changed

In the cited paper "Frobenius Pseudoprimes" p. 876, the second condition on being an "extra strong" Lucas pseudoprime reads:

V_{2^ts} ≡ 0 mod n, where 0 ≤ t < r-1

When V == 0 and s == 1, by the above definition ("s" is "r" there), the number should not be declared as a pseudoprime. Hence the change is needed.

Other comments

Release Notes

  • ntheory
    • Corrects the condition for "Extra strong" Lucas pseudoprime

@codecov
Copy link

codecov bot commented Mar 24, 2020

Codecov Report

Merging #18946 into master will decrease coverage by 0.002%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##            master    #18946       +/-   ##
=============================================
- Coverage   75.741%   75.738%   -0.003%     
=============================================
  Files          647       647               
  Lines       168584    168599       +15     
  Branches     39723     39728        +5     
=============================================
+ Hits        127688    127695        +7     
- Misses       35344     35347        +3     
- Partials      5552      5557        +5

@Arpan612
Copy link
Contributor Author

@sylee957 Sir, could you please review the PR.

@sylee957
Copy link
Member

I think that there is no way to make this change visible.
If s is 1, V seems like always not divisible by n. But I haven't got through how to prove this.

@Arpan612
Copy link
Contributor Author

Arpan612 commented Mar 24, 2020

@sylee957 Sir, do you expect a change in PR? Or at least could you share some documents for me to explore on this issue?

@sylee957
Copy link
Member

I think that you should squash the commits because there are more redundant commits than the code itself.

My comment above is about the difficulty to getting the counterexample that is fixed.

@sympy-bot
Copy link

sympy-bot commented Mar 25, 2020

Hi, I am the SymPy bot (v158). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
"Extra strong" Lucas pseudoprime 2nd condition corrected
#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #18921 

#### Brief description of what is fixed or changed
In the cited paper "Frobenius Pseudoprimes" p. 876, the second condition on being an "extra strong" Lucas pseudoprime reads:

V_{2^ts} ≡ 0 mod n, where 0 ≤ t < r-1

When V == 0 and s == 1, by the above definition ("s" is "r" there), the number should not be declared as a pseudoprime. Hence the change is needed.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* ntheory
   * Corrects the condition for "Extra strong" Lucas pseudoprime
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@czgdp1807
Copy link
Member

I have cleared the commits. Just make some minor changes so that your name is recorded as well in the git history.

@Arpan612 Arpan612 closed this Mar 25, 2020
@Arpan612 Arpan612 reopened this Mar 25, 2020
@czgdp1807
Copy link
Member

Is there any test case that can be added for this fix?

@Arpan612
Copy link
Contributor Author

Arpan612 commented Mar 25, 2020

@czgdp1807 Sir, could you please tell me how did you manage the squash commit? My attempt was continuously showing compatibility issues.

@Arpan612
Copy link
Contributor Author

@czgdp1807 Sir, I have added the test cases.

@mohitacecode
Copy link
Contributor

you have to add test in test_primetest.py file.

@mohitacecode
Copy link
Contributor

This is not how you should write test take a look at test_primetest.py you will get it.

@Arpan612
Copy link
Contributor Author

@mohitacecode Got it. Working on it.

@Arpan612
Copy link
Contributor Author

@sylee957 @czgdp1807 Could you please review the changes?

Changes

Test cases added

Tests added
@Arpan612 Arpan612 requested a review from sylee957 March 26, 2020 14:16
@Arpan612
Copy link
Contributor Author

@sylee957 Thank you for the correction.

@sylee957 sylee957 merged commit 9640007 into sympy:master Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ntheory/primetest/is_extra_strong_lucas_prp edge case
5 participants