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

Use PARI when computing the Tate pairing #36994

Merged
merged 8 commits into from
Jan 22, 2024

Conversation

GiacomoPope
Copy link
Contributor

@GiacomoPope GiacomoPope commented Jan 2, 2024

To match weil_pairing() this pull request now uses PARI to compute the non-reduced Tate pairing for elliptic curves over finite fields.

This has been included for efficiency, with PARI offering a significant performance speed up:

sage: GF(65537^2).inject_variables()
Defining z2
sage: E = EllipticCurve(GF(65537^2), [0,1])
sage: P = E(22, 28891)
sage: Q = E.random_point()
sage: # Old timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
1.4 ms ± 66 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: # New timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
50.8 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

@GiacomoPope
Copy link
Contributor Author

I'm now not actually convinced that the tate_pairing() is ever expected to be called over non-finite fields... I need to read more literature but if we only consider E/K with characteristic(K) > 0, then we can simply always use a pari call and simplify this function

@GiacomoPope GiacomoPope changed the title Allow the use of PARI when appropriate for computing the Tate pairing Use PARI to computing the Tate pairing Jan 3, 2024
@GiacomoPope
Copy link
Contributor Author

Rather than allow Pari to be used when the field is finite, we now assume that the field is finite for the reduced tate pairing, return a NotImplemented error when the field is not finite and always use Pari for the performance boost.

@GiacomoPope GiacomoPope changed the title Use PARI to computing the Tate pairing Use PARI when computing the Tate pairing Jan 4, 2024
Copy link
Member

@JohnCremona JohnCremona left a comment

Choose a reason for hiding this comment

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

It's a good idea to use pari where it has the functionality, as here (but was not the case back in 2011 I think).

I only made two suggestions, one a trivial one about wording and the other in response to the TODO.

src/sage/schemes/elliptic_curves/ell_point.py Outdated Show resolved Hide resolved
ePQ = pari.elltatepairing(E, P, Q, n)
# TODO: if n or k is chosen badly, this could error, should we
# handle this explicitly by ensuring n divides q^k - 1?
exp = Integer((q**k - 1)/n)
Copy link
Member

Choose a reason for hiding this comment

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

I think it is a good idea to check that n divides q^k-1, for example by computing mod(q,n)^k, but only if q has been explicitly supplied and with a suitable error message if the condition fails. So do this test in the code block which computes q perhaps.

@GiacomoPope
Copy link
Contributor Author

I have fixed the English in one of the ValueErrors and also added a new check that n divides (q^k - 1) which is only checked when q is not None (The case where a user supplies this information).

Copy link

github-actions bot commented Jan 9, 2024

Documentation preview for this PR (built with commit d118f1c; changes) is ready! 🎉

Copy link
Member

@JohnCremona JohnCremona left a comment

Choose a reason for hiding this comment

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

Good, thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
    
To match `weil_pairing()` this pull request now uses PARI to compute the
non-reduced Tate pairing for elliptic curves over finite fields.

This has been included for efficiency, with PARI offering a significant
performance speed up:

```py
sage: GF(65537^2).inject_variables()
Defining z2
sage: E = EllipticCurve(GF(65537^2), [0,1])
sage: P = E(22, 28891)
sage: Q = E.random_point()
sage: # Old timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
1.4 ms ± 66 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: # New timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
50.8 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops
each)
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#36994
Reported by: Giacomo Pope
Reviewer(s): John Cremona
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
    
To match `weil_pairing()` this pull request now uses PARI to compute the
non-reduced Tate pairing for elliptic curves over finite fields.

This has been included for efficiency, with PARI offering a significant
performance speed up:

```py
sage: GF(65537^2).inject_variables()
Defining z2
sage: E = EllipticCurve(GF(65537^2), [0,1])
sage: P = E(22, 28891)
sage: Q = E.random_point()
sage: # Old timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
1.4 ms ± 66 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: # New timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
50.8 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops
each)
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#36994
Reported by: Giacomo Pope
Reviewer(s): John Cremona
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
    
To match `weil_pairing()` this pull request now uses PARI to compute the
non-reduced Tate pairing for elliptic curves over finite fields.

This has been included for efficiency, with PARI offering a significant
performance speed up:

```py
sage: GF(65537^2).inject_variables()
Defining z2
sage: E = EllipticCurve(GF(65537^2), [0,1])
sage: P = E(22, 28891)
sage: Q = E.random_point()
sage: # Old timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
1.4 ms ± 66 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
sage: # New timing
sage: %timeit P.tate_pairing(Q, 7282, 2)
50.8 µs ± 674 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops
each)
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#36994
Reported by: Giacomo Pope
Reviewer(s): John Cremona
@vbraun vbraun merged commit 5117610 into sagemath:develop Jan 22, 2024
19 checks passed
@GiacomoPope GiacomoPope deleted the pari_tate_pairing branch March 6, 2024 16:23
@mkoeppe mkoeppe added this to the sage-10.3 milestone Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants