Add Tonelli-Shanks algorithm for modular square roots - #15144
Conversation
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
| from __future__ import annotations | ||
|
|
||
|
|
||
| def legendre_symbol(n: int, p: int) -> int: |
There was a problem hiding this comment.
Please provide descriptive name for the parameter: n
Please provide descriptive name for the parameter: p
| return ls if ls <= 1 else -1 | ||
|
|
||
|
|
||
| def tonelli_shanks(n: int, p: int) -> tuple[int, int]: |
There was a problem hiding this comment.
Please provide descriptive name for the parameter: n
Please provide descriptive name for the parameter: p
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
| from __future__ import annotations | ||
|
|
||
|
|
||
| def legendre_symbol(n: int, p: int) -> int: |
There was a problem hiding this comment.
Please provide descriptive name for the parameter: n
Please provide descriptive name for the parameter: p
| return ls if ls <= 1 else -1 | ||
|
|
||
|
|
||
| def tonelli_shanks(n: int, p: int) -> tuple[int, int]: |
There was a problem hiding this comment.
Please provide descriptive name for the parameter: n
Please provide descriptive name for the parameter: p
There was a problem hiding this comment.
Pull request overview
This PR adds a new educational implementation of the Tonelli–Shanks algorithm to compute modular square roots (x^2 \equiv n \pmod p) for an odd prime modulus, along with a helper for computing the Legendre symbol via Euler’s criterion.
Changes:
- Added
legendre_symbol()helper using Euler’s criterion. - Added
tonelli_shanks()implementation with doctests and references for modular square roots modulo an odd prime.
Suppressed comments (2)
maths/tonelli_shanks.py:86
- The error message and doctests claim
pis validated as an odd prime, but the code only rejects even/<=2 values. Consider rewording the message/doctests to avoid implying primality is checked (or add a prime check).
if p <= 2 or p % 2 == 0:
msg = f"Modulus p must be an odd prime (got {p})."
raise ValueError(msg)
maths/tonelli_shanks.py:112
- The loop that searches for a quadratic non-residue (
z) is unbounded. Sincepis not actually validated as prime, invalid inputs can cause this to run indefinitely. Add a bound (z < p) and fail fast with a helpful error if no non-residue is found.
# Case 2: Search for a quadratic non-residue z modulo p
z = 2
while legendre_symbol(z, p) != -1:
z += 1
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Raises: | ||
| ValueError: If p is not an odd prime >= 3. | ||
| ValueError: If n is not a quadratic residue modulo p. | ||
|
|
Describe your change:
Added the Tonelli-Shanks Algorithm in$x^2 \equiv n \pmod p$ ) for an odd prime $p$ . Also includes
maths/tonelli_shanks.pyto compute modular square roots (legendre_symbolcalculation using Euler's criterion.Checklist: