#24075 closed defect (fixed)
doctest failure in schemes/elliptic_curves/heegner.py
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-8.1 |
Component: | elliptic curves | Keywords: | |
Cc: | Merged in: | ||
Authors: | Frédéric Chapoton | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | 261e255 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
On sage 8.1.beta8 several people got
sage -t --long src/sage/schemes/elliptic_curves/heegner.py ********************************************************************** File "src/sage/schemes/elliptic_curves/heegner.py", line 6622, in sage.schemes.elliptic_curves.heegner.heegner_index Failed example: E.heegner_index(-8) Expected: Traceback (most recent call last): ... RuntimeError: ... Got: 1.00000? **********************************************************************
See original report on the sage-release thread.
Change History (15)
comment:1 Changed 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
- Component changed from documentation to elliptic curves
comment:4 follow-up: ↓ 5 Changed 5 years ago by
Nice analysis. So this is caused by #23962.
I would like to use a different curve which is not in any database.
comment:5 in reply to: ↑ 4 Changed 5 years ago by
comment:6 Changed 5 years ago by
Try this:
sage: E = EllipticCurve([1,-1,0,-1228,-16267]) sage: E.conductor() 99982889 sage: E.heegner_index(-8) Unable to compute the rank with certainty (lower bound=0). This could be because Sha(E/Q)[2] is nontrivial. Try calling something like two_descent(second_limit=13) on the curve then trying this command again. You could also try rank with only_use_mwrank=False. --------------------------------------------------------------------------- RuntimeError Traceback (most recent call last) <ipython-input-3-367e86799b13> in <module>() ----> 1 E.heegner_index(-Integer(8)) /usr/local/sage/sage-2/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/heegner.pyc in heegner_index(self, D, min_p, prec, descent_second_limit, verbose_mwrank, check_rank) 6522 raise ArithmeticError("Discriminant (=%s) must be a fundamental discriminant that satisfies the Heegner hypothesis."%D) 6523 -> 6524 if check_rank and self.rank() >= 2: 6525 return rings.infinity 6526 /usr/local/sage/sage-2/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/ell_rational_field.pyc in rank(self, use_database, verbose, only_use_mwrank, algorithm, proof) 2120 print("with only_use_mwrank=False.") 2121 del E.__mwrank_curve -> 2122 raise RuntimeError('Rank not provably correct.') 2123 else: 2124 misc.verbose("Warning -- rank not proven correct", level=1) RuntimeError: Rank not provably correct. sage: E.heegner_index(-8, descent_second_limit=16, check_rank=False) 2.00000?
I only put the conductor line in to show that this is beyond the database. (Sorry, it is in Stein-Watkins.) Note that the correct Heegner index is 2 not 1 as in the other example.
I'll look for an example outside SW if you want but not right now.
comment:7 Changed 5 years ago by
BTW even if the huge SW database is installed this function would not be affected, so this example should be good. Finding examples is not that easy: E should have rank 1 and the quadratic twist used rank 0. At least one of those ranks should fail to be computed by default but succeed with the additional parameters given.
If people are happy with the new example I already posted, it would be great if someone else would make the patch; I'll review it. It need not be a blocker though, surely?
comment:8 Changed 5 years ago by
It definitely is a blocker: an optional package is breaking a doctest.
comment:9 Changed 5 years ago by
- Branch set to u/chapoton/24075
- Commit set to 261e255bf02cdb79419358f1fc3aaeb515b4f0fd
- Status changed from new to needs_review
comment:10 Changed 5 years ago by
- Reviewers set to John Cremona
- Status changed from needs_review to needs_work
This worked for me on a system with the optional database attached. I assume that the patchbots are testing with that -- I cannot interpret the variety of results various patchbots get but since the issue was to have an example which behaves the right way even with the optional database, this looks good.
comment:11 Changed 5 years ago by
- Status changed from needs_work to positive_review
comment:12 Changed 5 years ago by
- Branch changed from u/chapoton/24075 to 261e255bf02cdb79419358f1fc3aaeb515b4f0fd
- Resolution set to fixed
- Status changed from positive_review to closed
comment:13 Changed 5 years ago by
- Commit 261e255bf02cdb79419358f1fc3aaeb515b4f0fd deleted
Hi John,
Just in case you want to have some help interpreting failing patchbots sometime in the future: on #23832 there is a list of known failures for patchbots, there is also a link to it called "metaticket for patchbot failures" on trac.sagemath.org .The reason we considered this ticket a blocker is because it was also causing patchbots to fail and thereby hindering the sage development process.
comment:14 Changed 5 years ago by
Note also that some patchbots are run with optional packages and some are not... the information is somewhere hidden in the log
Using --optional=ccache,gdb,mpir,python2,sage
versus
Using --optional=4ti2,autotools,benzene,bliss,boost,buckygen,cmake,coxeter3,cryptominisat,csdp,d3js,database_jones_numfield,database_kohel,database_mutation_class,database_odlyzko_zeta,database_pari,database_stein_watkins,database_stein_watkins_mini,database_symbolic_data,deformation,dot2tex,frobby,gambit,gap_jupyter,gp2c,igraph,latte_int,libbraiding,libhomfly,libogg,lidia,lrslib,mcqd,meataxe,mpfrcx,mpir,nose,notedown,openssl,ore_algebra,pandoc_attributes,pandocfilters,pari_jupyter,plantri,pysingular,python2,python_igraph,pyx,qhull,saclib,sage,scons,singular_jupyter,sirocco,tdlib,termcap,tides,topcom
comment:15 Changed 5 years ago by
OK, thanks both.
This test uses the elliptic curve with Cremona label 274627a1 which is in the optional database, and has rank 1. Anyone running the test without the database will be computing the rank and that cannot be done using defaults (i.e. just doing E.rank()) as the generator is quite large. Those people seeing this fault probably have the database installed, so that the rank is set to 1 when te curve is created.
We can switch the test to a curve of conductor >400000, or manually set its rank to 1 using E._set_rank(). Or we can make this test optional on have the large database installed.