Schoof's Algorithm #19071

abhinav-anand-addepar · 2020-04-04T19:06:38Z

ntheory
- Implement schoof's algorithm

sympy-bot · 2020-04-04T19:06:50Z

✅

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:

ntheory
- Implement schoof's algorithm (#19071 by @abhinav28071999)

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.

<!-- BEGIN RELEASE NOTES -->
* ntheory
  * Implement schoof's algorithm
<!-- END RELEASE NOTES -->

codecov · 2020-04-04T20:29:48Z

Codecov Report

Merging #19071 into master will increase coverage by 0.088%.
The diff coverage is 89.454%.

@@              Coverage Diff              @@
##            master    #19071       +/-   ##
=============================================
+ Coverage   75.730%   75.819%   +0.088%     
=============================================
  Files          647       650        +3     
  Lines       168733    169202      +469     
  Branches     39767     39873      +106     
=============================================
+ Hits        127783    128288      +505     
+ Misses       35389     35338       -51     
- Partials      5561      5576       +15

sympy/ntheory/elliptic_curve.py

asmeurer · 2020-04-13T19:34:18Z

sympy/ntheory/elliptic_curve.py

+                    short_com["p9"] = exp(y2, p, fl)
+                    short_com["alpha2"] = alpha**2 % fl
+                    short_com["beta2"] = beta**2 % fl
+                    short_com["p5p4"] = short_com["p4"]*short_com["p5"] % fl


It think it could be cleaner to just use variables for these, and pass them through to the various functions as parameters.

I will make the changes.

asmeurer · 2020-04-13T19:37:04Z

sympy/ntheory/elliptic_curve.py

+        References
+        ==========
+
+        [1] http://jjmcgee.asp.radford.edu/SchoofsAlgorithm06.pdf


Is the code here all based on this reference or are there other references too? If the code here is based on some pseudocode from a paper, that is fine, but you should indicate that so that it is clear where we can look to understand the code.

Yes this is based on the pseudocode from the referenced paper.

asmeurer · 2020-04-13T19:37:45Z

I think this needs a lot more tests. We should aim for full coverage of the code, particularly all the different if blocks. Also if there is any way to verify the that the tests themselves are correct we should do that.

czgdp1807 · 2020-04-29T06:54:25Z

sympy/ntheory/elliptic_curve.py

 from .factor_ import divisors
 from .residue_ntheory import polynomial_congruence



+


Suggested change

czgdp1807 · 2020-04-29T06:55:00Z

sympy/ntheory/elliptic_curve.py

+        X = symbols('X')
+        Y = symbols('Y')
+        div = {}
+        if domain != QQ:


Suggested change

if domain != QQ:

if domain is not QQ:

QQ should be a unique object.

czgdp1807 · 2020-04-29T06:55:39Z

sympy/ntheory/elliptic_curve.py

+        Division Polynomials of an elliptic curve over finite fields.
+
+        """
+        from sympy import simplify


Suggested change

from sympy import simplify

from sympy import _simplify

czgdp1807 · 2020-04-29T06:57:00Z

sympy/ntheory/elliptic_curve.py

+        l = 1
+        i = 1
+        p = self.modulus
+        X = self.X
+        Y = self.Y
+        L = []
+        m = 1
+        f2 = {}
+        y2 = poly(X**3 + self._a4*X+self._a6, X, modulus=p)


Suggested change

l = 1

i = 1

p = self.modulus

X = self.X

Y = self.Y

L = []

m = 1

f2 = {}

y2 = poly(X**3 + self._a4*X+self._a6, X, modulus=p)

l, i, m = 1, 1, 1

p, X, Y = self.modulus, self.X, self.Y

L, f2 = [], {}

y2 = poly(X**3 + self._a4*X+self._a6, X, modulus=p)

czgdp1807 · 2020-04-29T06:58:16Z

sympy/ntheory/elliptic_curve.py

+            p1 = short_com["p3"]
+            p1 = poly(X, X, modulus=p) - p1


Suggested change

p1 = short_com["p3"]

p1 = poly(X, X, modulus=p) - p1

p1 = poly(X, X, modulus=p) - short_com["p3"]

czgdp1807 · 2020-04-29T06:58:43Z

sympy/ntheory/elliptic_curve.py

+            return  p_16 % f(l)
+
+
+        def f(k):


A more descriptive function name?

czgdp1807 · 2021-01-02T06:45:12Z

@abh2k Was this PR a part of your GSoC project?

abhinav-anand-addepar · 2021-01-02T21:11:53Z

@abh2k Was this PR a part of your GSoC project?

No.

added division polynomials for elliptic curve

789a4a5

czgdp1807 reviewed Apr 5, 2020

View reviewed changes

sympy/ntheory/elliptic_curve.py Show resolved Hide resolved

czgdp1807 added the ntheory label Apr 5, 2020

abhinav-anand-addepar added 4 commits April 7, 2020 03:27

Added schoof algorithm

5111015

removed slow tests

7eec342

improved code quality

5827e59

removed unused import

58793b2

abhinav-anand-addepar closed this Apr 7, 2020

abhinav-anand-addepar reopened this Apr 7, 2020

abhinav-anand-addepar changed the title ~~[WIP] Schoof's Algorithm~~ Schoof's Algorithm Apr 8, 2020

asmeurer reviewed Apr 13, 2020

View reviewed changes

czgdp1807 reviewed Apr 29, 2020

View reviewed changes

sympy/ntheory/elliptic_curve.py

from .factor_ import divisors

from .residue_ntheory import polynomial_congruence

Copy link

Member

czgdp1807 Apr 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

czgdp1807 reviewed Apr 29, 2020

View reviewed changes

sympy/ntheory/elliptic_curve.py

return p_16 % f(l)

def f(k):

Copy link

Member

czgdp1807 Apr 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more descriptive function name?

czgdp1807 added the Please take over label Jan 2, 2021

github-actions bot added the Merge conflict label Oct 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Schoof's Algorithm #19071

Schoof's Algorithm #19071

abhinav-anand-addepar commented Apr 4, 2020

sympy-bot commented Apr 4, 2020

codecov bot commented Apr 4, 2020 •

edited

asmeurer Apr 13, 2020

abhinav-anand-addepar Apr 15, 2020

asmeurer Apr 13, 2020

abhinav-anand-addepar Apr 15, 2020

asmeurer commented Apr 13, 2020

czgdp1807 Apr 29, 2020

czgdp1807 Apr 29, 2020

czgdp1807 Apr 29, 2020

czgdp1807 Apr 29, 2020

czgdp1807 Apr 29, 2020

czgdp1807 Apr 29, 2020

czgdp1807 commented Jan 2, 2021

abhinav-anand-addepar commented Jan 2, 2021

		from .factor_ import divisors
		from .residue_ntheory import polynomial_congruence

	p1 = short_com["p3"]
	p1 = poly(X, X, modulus=p) - p1
	p1 = poly(X, X, modulus=p) - short_com["p3"]

Schoof's Algorithm #19071

Are you sure you want to change the base?

Schoof's Algorithm #19071

Conversation

abhinav-anand-addepar commented Apr 4, 2020

sympy-bot commented Apr 4, 2020

codecov bot commented Apr 4, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmeurer commented Apr 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

czgdp1807 commented Jan 2, 2021

abhinav-anand-addepar commented Jan 2, 2021

codecov bot commented Apr 4, 2020 •

edited