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

remove experimental warning for composite elliptic-curve isogenies #34409

Closed
yyyyx4 opened this issue Aug 22, 2022 · 15 comments
Closed

remove experimental warning for composite elliptic-curve isogenies #34409

yyyyx4 opened this issue Aug 22, 2022 · 15 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Aug 22, 2022

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

@yyyyx4 yyyyx4 added this to the sage-9.7 milestone Aug 22, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

Changed commit from 23f9b0c to fd50425

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2022

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

This comment has been minimized.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 22, 2022

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
Copy link
Collaborator

kwankyu commented Aug 22, 2022

Reviewer: Kwankyu Lee

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 22, 2022

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
Copy link
Collaborator

kwankyu commented Aug 22, 2022

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
Copy link
Member Author

yyyyx4 commented Aug 23, 2022

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
Copy link
Collaborator

kwankyu commented Aug 23, 2022

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
Copy link
Member

vbraun commented Aug 29, 2022

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

9c80ec3 Trac #34377: Improvements to ImageSet

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

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

1749b85 Trac #34373: Implement multimajor index for permutations

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

e874714 Trac #34371: support factoring polynomials modulo prime powers

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

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

8a41fd6 Trac #32324: Lazy Taylor Series

ae8a36d Trac #32887: update sagetex to version 3.6.1

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

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

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

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

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

efc1cd0 Trac #34330: bug in LaurentPolynomial_univariate.quo_rem

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

5a0647f Trac #34308: use libgap for abelian subgroups

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

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

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

6b373e6 Trac #34292: Group algebra bug

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

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

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

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

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

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

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

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

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

fe976b3 Trac #34301: Remove claims that Cygwin is supported

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

e96e201 Trac #34188: provide hash for decorated permutations

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

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

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

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

2c4005d Trac #31276: Tensor Product Method for FiniteRankFreeModule

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

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

a444241 Trac #29717: Cubic Hecke Algebras

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

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

0b2cecc Trac #34324: Fix deprecated import of instancedoc

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

a5bf710 Trac #34421: Fix timeout in jupyter_jsmol installation

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

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

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

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

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

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

08b5040 Trac #34294: SimplicialComplex: Add method is_subcomplex

9b5044a Trac #34289: minor tweaks in the doc

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

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

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

a965858 Trac #34157: Meta ticket: fix docstring markups

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

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

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

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

12be2d9 Updated SageMath version to 9.7.beta8

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

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2022

comment:10

Let's wait for the next beta.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2022

Changed commit from fd50425 to b7663b4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2022

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
Copy link
Member Author

yyyyx4 commented Aug 31, 2022

comment:12

Trivial rebase.

@vbraun
Copy link
Member

vbraun commented Sep 27, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants