Re: [sage-release] Re: Sage 10.1.beta3 released
Maybe I'm being stupid, but I don't think the given list of generators is complete. The inverse of a generator is also a generator, so aren't there 8 generators, not 4? (Sorry I couldn't find the PR, to put this comment there.) On Friday, June 16, 2023 at 2:57:38 AM UTC-6 John Cremona wrote: > I'll review the new PR, though I probably should have seen this problem > arising with the original one. > > John > > On Fri, 16 Jun 2023 at 09:30, Dima Pasechnik wrote: > >> You can't reopen PRs, I think, you can certainly reopen issues. >> >> No need to fill in the longish PR boilerplate, just remove it and write >> what it does, mentioning #35626 >> >> >> >> On Fri, 16 Jun 2023, 09:08 chris wuthrich, >> wrote: >> >>> I replaced old tests with "random" by new ones with "random" (and forgot >>> these two). Some other tests do check consistency. >>> >>> All that the function really does is converting a pari output back into >>> a point on the elliptic curve. Therefore the test checks if pari runs >>> without error and if the returned is a point on the curve. Checking if it >>> is a generator modulo torsion would be checking if pari gets this right. Is >>> that what we should do? >>> >>> But this discussing should happen on a pull request. I repeat my >>> question: Do I open a new one or is the faulty initial one reopened? (Trac >>> tickets used to be reopened, that is why I ask). >>> >>> Chris >>> >>> On Friday, 16 June 2023 at 01:39:46 UTC+1 tsc...@ucdavis.edu wrote: >>> I don't think it should be random; that has the chance of hiding actual bugs. I think we should check that it belongs to a particular set (specifically, the one from John's comment). Best, Travis On Thursday, June 15, 2023 at 5:15:52 AM UTC+9 chris wuthrich wrote: > I agree with John. Lines 2380 and 2388 need a "random output" for that > reason, like the other calls of that function have already. > Is this a new pull request or is the old #35626 > https://github.com/sagemath/sage/pull/35626 opened again for > corrections? > > Chris > > On Wednesday, 14 June 2023 at 17:49:21 UTC+1 John Cremona wrote: > >> On Tue, 13 Jun 2023 at 17:06, John Cremona >> wrote: >> >>> The elliptic curve failure might be a consequence of the recently >>> merged PR #35626 about using libpari to compute ranks and generators. >>> >>> I'll take a look to at least see if the output you get is >>> mathematically correct -- generators are not unique. >>> >> >> I was right. This curve has rank 1 and torsion (Z/2Z)^2 so if P is >> one generator (of infinte order) then there are 3 other, P+T for T a >> point >> of order 2. The one your are getting is one of these. >> >> So we conclude that we cannot reply on pari always returning the same >> generator despite random seeds etc being fixed in doctests. I don't >> know >> why that is, but if it is the case then this (and other similar) >> doctests >> will have to be written accordingly. >> >> >>> >>> John >>> >>> >>> On Tue, 13 Jun 2023, 17:00 Emmanuel Charpentier, < >>> emanuel.c...@gmail.com> wrote: >>> On Debian testing running on core i7 + 16 GB RAM, upgrading 10.1.beta2 to 10.1.beta3 and rinning ptestlong gives 3 permanent failures : -- sage -t --long --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed sage -t --long --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 src/sage/coding/linear_code.py # 2 doctests failed sage -t --long --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 src/sage/coding/code_constructions.py # 1 doctest failed -- The last two have been already reported a few times, and seem cosmetic. The first one is new and seemingly *not* cosmetic : charpent@zen-book-flip:/usr/local/sage-10$ sage -t --long --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed Running doctests with ID 2023-06-13-17-49-08-8dac2a23. Git branch: develop Git ref: 10.1.beta0-453-g443b7549ad Running with SAGE_LOCAL='/usr/local/sage-10/local' and SAGE_VENV='/usr/local/sage-10/local/var/lib/sage/venv-python3.11' Using --optional=debian,dot2tex,fricas,gap_jupyter,gap_packages,libsemigroups,msolve,pip,pysingular,sage,sage_spkg,singular_jupyter
Re: [sage-release] Re: Sage 10.1.beta3 released
I'll review the new PR, though I probably should have seen this problem arising with the original one. John On Fri, 16 Jun 2023 at 09:30, Dima Pasechnik wrote: > You can't reopen PRs, I think, you can certainly reopen issues. > > No need to fill in the longish PR boilerplate, just remove it and write > what it does, mentioning #35626 > > > > On Fri, 16 Jun 2023, 09:08 chris wuthrich, > wrote: > >> I replaced old tests with "random" by new ones with "random" (and forgot >> these two). Some other tests do check consistency. >> >> All that the function really does is converting a pari output back into a >> point on the elliptic curve. Therefore the test checks if pari runs without >> error and if the returned is a point on the curve. Checking if it is a >> generator modulo torsion would be checking if pari gets this right. Is that >> what we should do? >> >> But this discussing should happen on a pull request. I repeat my >> question: Do I open a new one or is the faulty initial one reopened? (Trac >> tickets used to be reopened, that is why I ask). >> >> Chris >> >> On Friday, 16 June 2023 at 01:39:46 UTC+1 tsc...@ucdavis.edu wrote: >> >>> I don't think it should be random; that has the chance of hiding actual >>> bugs. I think we should check that it belongs to a particular set >>> (specifically, the one from John's comment). >>> >>> Best, >>> Travis >>> >>> >>> On Thursday, June 15, 2023 at 5:15:52 AM UTC+9 chris wuthrich wrote: >>> I agree with John. Lines 2380 and 2388 need a "random output" for that reason, like the other calls of that function have already. Is this a new pull request or is the old #35626 https://github.com/sagemath/sage/pull/35626 opened again for corrections? Chris On Wednesday, 14 June 2023 at 17:49:21 UTC+1 John Cremona wrote: > On Tue, 13 Jun 2023 at 17:06, John Cremona > wrote: > >> The elliptic curve failure might be a consequence of the recently >> merged PR #35626 about using libpari to compute ranks and generators. >> >> I'll take a look to at least see if the output you get is >> mathematically correct -- generators are not unique. >> > > I was right. This curve has rank 1 and torsion (Z/2Z)^2 so if P is one > generator (of infinte order) then there are 3 other, P+T for T a point of > order 2. The one your are getting is one of these. > > So we conclude that we cannot reply on pari always returning the same > generator despite random seeds etc being fixed in doctests. I don't know > why that is, but if it is the case then this (and other similar) doctests > will have to be written accordingly. > > >> >> John >> >> >> On Tue, 13 Jun 2023, 17:00 Emmanuel Charpentier, < >> emanuel.c...@gmail.com> wrote: >> >>> On Debian testing running on core i7 + 16 GB RAM, upgrading >>> 10.1.beta2 to 10.1.beta3 and rinning ptestlong gives 3 permanent >>> failures : >>> -- >>> sage -t --long --warn-long 207.1 >>> --random-seed=291812591553963182024849035945523427319 >>> src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest >>> failed >>> sage -t --long --warn-long 207.1 >>> --random-seed=291812591553963182024849035945523427319 >>> src/sage/coding/linear_code.py # 2 doctests failed sage -t --long >>> --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 >>> src/sage/coding/code_constructions.py # 1 doctest failed >>> -- >>> >>> The last two have been already reported a few times, and seem >>> cosmetic. The first one is new and seemingly *not* cosmetic : >>> charpent@zen-book-flip:/usr/local/sage-10$ sage -t --long >>> --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 >>> src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest >>> failed >>> Running doctests with ID 2023-06-13-17-49-08-8dac2a23. Git branch: >>> develop >>> Git ref: 10.1.beta0-453-g443b7549ad Running with >>> SAGE_LOCAL='/usr/local/sage-10/local' and >>> SAGE_VENV='/usr/local/sage-10/local/var/lib/sage/venv-python3.11' Using >>> --optional=debian,dot2tex,fricas,gap_jupyter,gap_packages,libsemigroups,msolve,pip,pysingular,sage,sage_spkg,singular_jupyter >>> Features to be detected: >>>
Re: [sage-release] Re: Sage 10.1.beta3 released
You can't reopen PRs, I think, you can certainly reopen issues. No need to fill in the longish PR boilerplate, just remove it and write what it does, mentioning #35626 On Fri, 16 Jun 2023, 09:08 chris wuthrich, wrote: > I replaced old tests with "random" by new ones with "random" (and forgot > these two). Some other tests do check consistency. > > All that the function really does is converting a pari output back into a > point on the elliptic curve. Therefore the test checks if pari runs without > error and if the returned is a point on the curve. Checking if it is a > generator modulo torsion would be checking if pari gets this right. Is that > what we should do? > > But this discussing should happen on a pull request. I repeat my question: > Do I open a new one or is the faulty initial one reopened? (Trac tickets > used to be reopened, that is why I ask). > > Chris > > On Friday, 16 June 2023 at 01:39:46 UTC+1 tsc...@ucdavis.edu wrote: > >> I don't think it should be random; that has the chance of hiding actual >> bugs. I think we should check that it belongs to a particular set >> (specifically, the one from John's comment). >> >> Best, >> Travis >> >> >> On Thursday, June 15, 2023 at 5:15:52 AM UTC+9 chris wuthrich wrote: >> >>> I agree with John. Lines 2380 and 2388 need a "random output" for that >>> reason, like the other calls of that function have already. >>> Is this a new pull request or is the old #35626 >>> https://github.com/sagemath/sage/pull/35626 opened again for >>> corrections? >>> >>> Chris >>> >>> On Wednesday, 14 June 2023 at 17:49:21 UTC+1 John Cremona wrote: >>> On Tue, 13 Jun 2023 at 17:06, John Cremona wrote: > The elliptic curve failure might be a consequence of the recently > merged PR #35626 about using libpari to compute ranks and generators. > > I'll take a look to at least see if the output you get is > mathematically correct -- generators are not unique. > I was right. This curve has rank 1 and torsion (Z/2Z)^2 so if P is one generator (of infinte order) then there are 3 other, P+T for T a point of order 2. The one your are getting is one of these. So we conclude that we cannot reply on pari always returning the same generator despite random seeds etc being fixed in doctests. I don't know why that is, but if it is the case then this (and other similar) doctests will have to be written accordingly. > > John > > > On Tue, 13 Jun 2023, 17:00 Emmanuel Charpentier, < > emanuel.c...@gmail.com> wrote: > >> On Debian testing running on core i7 + 16 GB RAM, upgrading >> 10.1.beta2 to 10.1.beta3 and rinning ptestlong gives 3 permanent >> failures : >> -- >> sage -t --long --warn-long 207.1 >> --random-seed=291812591553963182024849035945523427319 >> src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed >> sage -t --long --warn-long 207.1 >> --random-seed=291812591553963182024849035945523427319 >> src/sage/coding/linear_code.py # 2 doctests failed sage -t --long >> --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 >> src/sage/coding/code_constructions.py # 1 doctest failed >> -- >> >> The last two have been already reported a few times, and seem >> cosmetic. The first one is new and seemingly *not* cosmetic : >> charpent@zen-book-flip:/usr/local/sage-10$ sage -t --long >> --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 >> src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed >> Running doctests with ID 2023-06-13-17-49-08-8dac2a23. Git branch: >> develop >> Git ref: 10.1.beta0-453-g443b7549ad Running with >> SAGE_LOCAL='/usr/local/sage-10/local' and >> SAGE_VENV='/usr/local/sage-10/local/var/lib/sage/venv-python3.11' Using >> --optional=debian,dot2tex,fricas,gap_jupyter,gap_packages,libsemigroups,msolve,pip,pysingular,sage,sage_spkg,singular_jupyter >> Features to be detected: >>
Re: [sage-release] Re: Sage 10.1.beta3 released
The original pull request was merged, so open a new one. Trac tickets were occasionally reopened but not after they were merged in a development release. -- You received this message because you are subscribed to the Google Groups "sage-release" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-release+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/sage-release/CAEcArF27yYr4UUZDYCizQEjoO1bwpO0f2sheFyDvGU-2g53s0w%40mail.gmail.com.
Re: [sage-release] Re: Sage 10.1.beta3 released
I replaced old tests with "random" by new ones with "random" (and forgot these two). Some other tests do check consistency. All that the function really does is converting a pari output back into a point on the elliptic curve. Therefore the test checks if pari runs without error and if the returned is a point on the curve. Checking if it is a generator modulo torsion would be checking if pari gets this right. Is that what we should do? But this discussing should happen on a pull request. I repeat my question: Do I open a new one or is the faulty initial one reopened? (Trac tickets used to be reopened, that is why I ask). Chris On Friday, 16 June 2023 at 01:39:46 UTC+1 tsc...@ucdavis.edu wrote: > I don't think it should be random; that has the chance of hiding actual > bugs. I think we should check that it belongs to a particular set > (specifically, the one from John's comment). > > Best, > Travis > > > On Thursday, June 15, 2023 at 5:15:52 AM UTC+9 chris wuthrich wrote: > >> I agree with John. Lines 2380 and 2388 need a "random output" for that >> reason, like the other calls of that function have already. >> Is this a new pull request or is the old #35626 >> https://github.com/sagemath/sage/pull/35626 opened again for corrections? >> >> Chris >> >> On Wednesday, 14 June 2023 at 17:49:21 UTC+1 John Cremona wrote: >> >>> On Tue, 13 Jun 2023 at 17:06, John Cremona wrote: >>> The elliptic curve failure might be a consequence of the recently merged PR #35626 about using libpari to compute ranks and generators. I'll take a look to at least see if the output you get is mathematically correct -- generators are not unique. >>> >>> I was right. This curve has rank 1 and torsion (Z/2Z)^2 so if P is one >>> generator (of infinte order) then there are 3 other, P+T for T a point of >>> order 2. The one your are getting is one of these. >>> >>> So we conclude that we cannot reply on pari always returning the same >>> generator despite random seeds etc being fixed in doctests. I don't know >>> why that is, but if it is the case then this (and other similar) doctests >>> will have to be written accordingly. >>> >>> John On Tue, 13 Jun 2023, 17:00 Emmanuel Charpentier, < emanuel.c...@gmail.com> wrote: > On Debian testing running on core i7 + 16 GB RAM, upgrading 10.1.beta2 > to 10.1.beta3 and rinning ptestlong gives 3 permanent failures : > -- > sage -t --long --warn-long 207.1 > --random-seed=291812591553963182024849035945523427319 > src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed > sage -t --long --warn-long 207.1 > --random-seed=291812591553963182024849035945523427319 > src/sage/coding/linear_code.py # 2 doctests failed sage -t --long > --warn-long 207.1 --random-seed=291812591553963182024849035945523427319 > src/sage/coding/code_constructions.py # 1 doctest failed > -- > > The last two have been already reported a few times, and seem > cosmetic. The first one is new and seemingly *not* cosmetic : > charpent@zen-book-flip:/usr/local/sage-10$ sage -t --long --warn-long > 207.1 --random-seed=291812591553963182024849035945523427319 > src/sage/schemes/elliptic_curves/ell_rational_field.py # 1 doctest failed > Running doctests with ID 2023-06-13-17-49-08-8dac2a23. Git branch: > develop > Git ref: 10.1.beta0-453-g443b7549ad Running with > SAGE_LOCAL='/usr/local/sage-10/local' and > SAGE_VENV='/usr/local/sage-10/local/var/lib/sage/venv-python3.11' Using > --optional=debian,dot2tex,fricas,gap_jupyter,gap_packages,libsemigroups,msolve,pip,pysingular,sage,sage_spkg,singular_jupyter > > Features to be detected: > 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,cvxopt,cvxopt,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_cubic_hecke,database_jones_numfield,database_knotinfo,dvipng,fpylll,gfan,graphviz,imagemagick,ipython,jupymake,kenzo,latte_int,lrcalc_python,lrslib,mcqd,meataxe,mpmath,msolve,nauty,networkx,numpy,palp,pandoc,pdf2svg,pdftocairo,pexpect,phitigra,pillow,plantri,polytopes_db,polytopes_db_4d,pplpy,primecountpy,ptyprocess,pynormaliz,pyparsing,python_igraph,requests,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.libs.gap,sage.libs.pari,sage.libs.singular,sage.misc.cython,sage.modules,sage.plot,sage.rings.finite_rings,sage.rings.function_field,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.rings.real_mpfr,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,scipy,singular,sphinx,sympy,tdlib > > Doctesting 1 file. sage -t --long --warn-long 207.1 > --random-seed=291812591553963182024849035945523427319 >
