sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.22k stars 425 forks source link

remove experimental warning for composite elliptic-curve isogenies #34409

Closed yyyyx4 closed 1 year ago

yyyyx4 commented 1 year ago

The code from #32744 was released with Sage 9.5 and has since been real-world tested by numerous people. It is exceedingly unlikely that we'll want to remove it, and I'm reasonably convinced that there are no disastrous bugs.

Therefore I propose to remove the experimental warning in the next release. See also #34303 comment:18.

In the future, we should also change the default composition method to EllipticCurveHom_composite: See #34410.

CC: @JohnCremona @kwankyu

Component: elliptic curves

Author: Lorenz Panny

Branch/Commit: b7663b4

Reviewer: Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/34409

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 1 year ago

Changed commit from 23f9b0c to fd50425

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 1 year ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

fd50425remove experimental warnings for composite elliptic-curve isogenies
yyyyx4 commented 1 year ago

Description changed:

--- 
+++ 
@@ -2,3 +2,4 @@

 Therefore I propose to remove the experimental warning in the next release. See also [#34303 comment:18](https://github.com/sagemath/sage/issues/34303#comment:18).

+In the future, we should also change the default composition method to `EllipticCurveHom_composite`: See #34410.
kwankyu commented 1 year ago
comment:4

Okay. But I want to express a concern:

I think it is a bad idea to use Sage library for sharing and testing code with many people. A trac ticket may be used to store your code for that purpose. But unreliable code in Sage even with an "experimental" tag is likely to waste other people's time and energy. I think this is like publishing a paper with an "experimental" tag.

Sorry if I misunderstood or am exaggerating. Or if you disagree, then this may be a topic for discussion in sage-devel.

kwankyu commented 1 year ago

Reviewer: Kwankyu Lee

yyyyx4 commented 1 year ago
comment:5

I agree that unreliable code shouldn't be included. The experimental warnings I added to some of my code are basically just there to allow us to tweak the API without going through deprecations. I only submit bigger chunks of code after running them myself for a while, and generally speaking I'm fairly confident that my patches are both useful and essentially correct.

Perhaps the meaning of the experimental warning is ambiguous: It could mean pretty much anything from "no idea what this computes exactly" to "we might rename this function later". I intended it more in the latter sense.

kwankyu commented 1 year ago
comment:6

The experimental warnings I added to some of my code are basically just there to allow us to tweak the API without going through deprecations.

There is an official way to do this:

https://doc.sagemath.org/html/en/developer/coding_in_python.html#experimental-unstable-code

Perhaps the meaning of the experimental warning is ambiguous.

I searched for the word "experimental" in the reference manual. There are 49 instances, perhaps with different meaning of "experimental". Perhaps your intended meaning would be the most harmless.

yyyyx4 commented 1 year ago
comment:7

Replying to @kwankyu:

The experimental warnings I added to some of my code are basically just there to allow us to tweak the API without going through deprecations.

There is an official way to do this:

https://doc.sagemath.org/html/en/developer/coding_in_python.html#experimental-unstable-code

I'm confused. Isn't this exactly what I did?

kwankyu commented 1 year ago
comment:8

Replying to @yyyyx4:

Replying to @kwankyu:

The experimental warnings I added to some of my code are basically just there to allow us to tweak the API without going through deprecations.

There is an official way to do this:

https://doc.sagemath.org/html/en/developer/coding_in_python.html#experimental-unstable-code

I'm confused. Isn't this exactly what I did?

Yes. I misunderstood your experimental warnings.

According to the doc, experimental warnings are mainly intended for unstable interface changes. From your ticket description, I thought you tagged the code "experimental" because you were not sure of reliability of the code, now you are because there has been no bug report. I questioned this use of experimental warnings.

vbraun commented 1 year ago
comment:9

Merge failure on top of:

62a2a3918e5 Trac #34388: a few more typos

98e62f750a2 Trac #34382: new bunch of fixed typos

1d668d449f3 Trac #34380: Free module does not correctly check domains

615e4c06faf Trac #34375: Replace sage.algebras.yangian.GeneratorIndexingSet with cartesian_product

e39c835bb04 Trac #34369: pycodestyle cleanup in modular/modform/element.py

dd87a2734c0 Trac #34366: remove (object) in sage_docbuild

a49b6304da9 Trac #34365: modernize super in numerical, plot, repl, topology

17aaa3063e0 Trac #34362: fix some E275

bf6f5b6c334 Trac #34361: some details in riemann_surface.py

cf1c91241e7 Trac #34357: pycodestyle cleanup in src/sage/graphs/generic_graph.py (part 3)

6c3eb949b8e Trac #34354: pycodestyle cleanup in src/sage/graphs/graph_database.py

91d2cc8af9b Trac #34353: Fix 4ti2 links and formatting

7cd06e3506d Trac #34351: triangulated surface of genus six

0a3195f3b83 Trac #34348: Add section on disputed styles on developer guide

283bb5a71b4 Trac #34336: base_ring is wrong for rational points in a projective space over a finite field

c9668eef858 Trac #34331: fix E251 in geometry and manifolds

dfcd81e37fa Trac #34329: fix E251 in interacts and interfaces

a5d2ac39d1a Trac #34325: fix E251 in categories/

6f862eb4cd6 Trac #34322: fix E251 in algebras/

231813e810e Trac #34321: fix E251 in combinat/sf

310703ae2f5 Trac #34319: pycodestyle cleanup in src/sage/graphs/generic_graph.py (part 2)

453162a89e6 Trac #34318: pycodestyle cleanup in src/sage/graphs/generic_graph.py (part 1)

4c4c4bb47fc Trac #34317: pycodestyle cleanup in src/sage/graphs/graph.py (part 3)

58e2a693a1d Trac #34316: pycodestyle cleanup in src/sage/graphs/graph.py (part 2)

5eddb25b6f8 Trac #34315: pycodestyle cleanup in src/sage/graphs/graph.py (part 1)

819c1b5ecc0 Trac #34312: pycodestyle cleanup in strongly_regular_db.pyx (part 2)

3f6eb379010 Trac #34311: pycodestyle cleanup in strongly_regular_db.pyx (part 1)

ed5b9d79d16 Trac #34310: pycodestyle cleanup in distance_regular.pyx (part 2)

1ea7ca6f7ba Trac #34309: pycodestyle cleanup in src/sage/graphs/generators/families.py

d54cb2b7dee Trac #34304: pep8 cleanup for one file in quadratic_forms

21bcbcad21f Trac #34299: refresh the Zariski-vanKampen file

4bb4177b0be Trac #34255: Make docker-in-docker available in Gitpod

9862c70756f Trac #34192: Remove imports from sage.rings.all in sage.calculus, functions, symbolic

8f685a4cefb Trac #34191: Remove imports from sage.rings.all in sage.modular

f8203dc3037 Trac #34190: Remove imports from sage.rings.all in sage.schemes

45224c914eb Trac #34128: Improve symbolic operators documentation

f4f200cb706 Trac #34075: pycodestyle cleanup in 5 files of src/sage/graphs/

caf3ef9b5db Trac #34074: pycodestyle cleanup in src/sage/graphs/graph_generators.py

36aa214c169 Trac #34054: Update documentation for EnumeratedFamily

7e8dc2caa7e Trac #33972: Another error in height_difference_bound

287293822d9 Trac #33953: Add some methods to projective morphisms (rational maps)

f927079fb4f Trac #33900: small enhancements to generic discrete logs

98fac39c5fd Trac #34425: configure vscode pycodestyle linter

57d6905ee40 Trac #34424: Common base class for FiniteRankFreeModule and TensorFreeModule

60421de93df Trac #34407: Refactor lazy series code so that it works over any graded ring

6ddea258611 Trac #34401: some details about MZV

78854caa4f0 Trac #34400: EnumeratedSets: Add method 'tuple', avoid making copies

60e10d2bf7d Trac #34393: add method "tensor_factors" to tensor products

9c80ec3d625 Trac #34377: Improvements to ImageSet

d83fb880415 Trac #34376: Set_object_enumerated should be in FiniteEnumeratedSets()

f84d7a4402d Trac #34374: Use cantor_product for Cartesian products of infinite enumerated sets

1749b85ad9d Trac #34373: Implement multimajor index for permutations

85b5ab977f2 Trac #34372: Make is_integral_domain() have the same signature

e874714965a Trac #34371: support factoring polynomials modulo prime powers

8a12a20563e Trac #34370: Add examples to Schubert polynomials documentation

a313529389d Trac #33671: Add devcontainer.json for development with VS Code in a Docker container

8a41fd62faa Trac #32324: Lazy Taylor Series

ae8a36d1191 Trac #32887: update sagetex to version 3.6.1

a41531c2417 Trac #34355: avoid constructing list of all base-field elements in QuaternionAlgebra_ab.modp_splitting_data()

b56e1c9dbf8 Trac #34352: Add comma in vscode.json config file

ad0536886ec Trac #34343: Speed up computing outside corners of partition

420bbe2527b Trac #34341: Fix bool(expr1 != expr2) for nontrivially equal expressions

5114e87c4cb Trac #34339: Speedup adding horizontal and vertical border strips

efc1cd01a1a Trac #34330: bug in LaurentPolynomial_univariate.quo_rem

d23fe5dd391 Trac #34326: ConvexSet_base.representative_point, Polyhedron_base.an_affine_basis for unbounded polyhedra

5a0647fc2bf Trac #34308: use libgap for abelian subgroups

87ea5fb3524 Trac #34306: Better use of graphs in src/sage/geometry/hyperplane_arrangement/library.py

9005c089e86 Trac #34303: Îlu algorithm: asymptotically faster elliptic-curve isogenies

4495944a9b7 Trac #34296: GH Actions: Upload wheels to PyPI

6b373e66d8d Trac #34292: Group algebra bug

42beee41cec Trac #34283: Prevent circular import of matrix space

cb618a34343 Trac #34281: defer primality and irreducibility testing in GF constructor until after caching

456c8fb7855 Trac #34261: Allow multiplication of a left and right noncommutative ideal

86ae68f849f Trac #34222: polymake: Upgrade to 4.7, remove deprecated Polymake_expect interface

cba438f7cac Trac #34219: Document that SageTeX is now in SAGE_ROOT/venv/share

0175f5a2c5c Trac #34186: Problem translating Fricas special function ellipticF to Sagemath

8f6d1ac645d Trac #33950: Add free and multigraded free resolutions with libSingular backend

894a2969311 Trac #23075: copy(CombinatorialFreeModule.Element) broken by #22632

163f7153798 Trac #33851: Script package _develop; improve installation instructions in the manual

fe976b3f0bb Trac #34301: Remove claims that Cygwin is supported

ea2758f6b1d Trac #34211: Fix bug due to a call to SSLContext() in src/sage/graphs/isgci.py

e96e201a66b Trac #34188: provide hash for decorated permutations

6f0dbf92b17 Trac #34138: Groebner bases for exterior algebras native to Sage

8a4672c14dc Trac #34116: add exact division of power series by coefficient

75d9213d64e Trac #32992: update ninja_build to 1.11.0, make it standard, add lower version bound

1c178e0b7f2 Trac #32369: Rewrite Clifford and exterior algebras to have a basis indexed by integers

2c4005d2cab Trac #31276: Tensor Product Method for FiniteRankFreeModule

3091ae98ae8 Trac #30300: sage.tensor.modules.free_module_basis: Make Basis_abstract a subclass of AbstractFamily

3f624d59f31 Trac #30235: Add construction methods to FiniteRankFreeModule, CombinatorialFreeModule and Cartesian products

a4442412904 Trac #29717: Cubic Hecke Algebras

ee070f20cd4 Trac #34442: Set version upper bound for setuptools: <64.0

24a6ab4b654 Trac #34367: README.md, installation guide: Mention cocalc Docker image instead of sagemath/sagemath image

0b2cecce13f Trac #34324: Fix deprecated import of instancedoc

758ce2c8bce Trac #30787: package modular_resolution: Split out from p_group_cohomology

a5bf7100496 Trac #34421: Fix timeout in jupyter_jsmol installation

bed45ffacc2 Trac #34360: curl configure --without-libmetalink no longer works

fb050b1204e Trac #34298: conda: 3d graphics do not show

20d7f182a25 Trac #34273: opensuse-tumbleweed: python3 build fails because of openssl

696bf78075d Trac #34270: .gitpod.yml: Do not hardcode the workspace name as sagetrac-mirror

5b7fc7e1d17 Trac #25675: Crosslinks to poset catalog, add documentation of sage.geometry.polyhedron.base* and combinatorial_polyhedron

4285f3c06e5 Trac #34295: Followup to #33627: fix documentation that mentions sage-gdb-commands

08b5040887f Trac #34294: SimplicialComplex: Add method is_subcomplex

9b5044a9347 Trac #34289: minor tweaks in the doc

75ae420b63b Trac #34288: some rst fixes in pyx files in coding, graphs and groups

2f94ddf09d3 Trac #34293: rubiks: Work around build failure with XCode

945c339e11b Trac #34291: Downgrade some optional packages to experimental in Sage 9.7

a965858a628 Trac #34157: Meta ticket: fix docstring markups

e9304778c6f Trac #34147: Implement the quantum Clifford algebra at a root of unity

75aaf289973 Trac #33596: PolyhedralComplex.plot(explosion_factor=1)

e9efc9c6349 Trac #33586: Triangulation.polyhedral_complex, boundary_simplicial_complex, boundary_polyhedral_complex

5247961337f Trac #34221: Backport PEP420 namespace package support from Cython 3

12be2d94c86 Updated SageMath version to 9.7.beta8

merge was not clean: conflicts in src/sage/schemes/elliptic_curves/ell_field.py

kwankyu commented 1 year ago
comment:10

Let's wait for the next beta.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 1 year ago

Changed commit from fd50425 to b7663b4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 1 year ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b7663b4remove experimental warnings for composite elliptic-curve isogenies
yyyyx4 commented 1 year ago
comment:12

Trivial rebase.

vbraun commented 1 year ago

Changed branch from public/remove_experimental_warning_for_composite_elliptic_curve_isogenies to b7663b4