Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#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) Commit:
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

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 20 months ago by vdelecroix

  • Description modified (diff)

comment:2 Changed 20 months ago by cremona

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.

comment:3 Changed 20 months ago by jdemeyer

  • Component changed from documentation to elliptic curves

comment:4 follow-up: Changed 20 months ago by jdemeyer

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 20 months ago by cremona

Replying to jdemeyer:

Nice analysis. So this is caused by #23962.

I would like to use a different curve which is not in any database.

I'll try to come up with one -- it should have rank 1 but E.rank() should fail to give that.

comment:6 Changed 20 months ago by cremona

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 20 months ago by cremona

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 20 months ago by vdelecroix

It definitely is a blocker: an optional package is breaking a doctest.

comment:9 Changed 20 months ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to u/chapoton/24075
  • Commit set to 261e255bf02cdb79419358f1fc3aaeb515b4f0fd
  • Status changed from new to needs_review

branch done


New commits:

261e255trac 24075 another example in heegner

comment:10 Changed 20 months ago by cremona

  • 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 20 months ago by cremona

  • Status changed from needs_work to positive_review

comment:12 Changed 20 months ago by vbraun

  • Branch changed from u/chapoton/24075 to 261e255bf02cdb79419358f1fc3aaeb515b4f0fd
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:13 Changed 20 months ago by mderickx

  • 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 20 months ago by vdelecroix

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 20 months ago by cremona

OK, thanks both.

Note: See TracTickets for help on using tickets.