Opened 9 years ago

Closed 8 years ago

#13084 closed enhancement (fixed)

Weierstrass form for toric elliptic curves

Reported by: vbraun Owned by: AlexGhitza
Priority: major Milestone: sage-5.11
Component: algebraic geometry Keywords:
Cc: novoselt, dimpase, mstreng Merged in: sage-5.11.beta2
Authors: Volker Braun Reviewers: Frédéric Chapoton, Dmitrii Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12553, #13451 Stopgaps:

Status badges

Description (last modified by vbraun)

This ticket implements the Weierstrass form (of the Jacobian) of anticanonical hypersurfaces in toric surfaces defined by reflexive polygons:

  sage: from sage.schemes.toric.weierstrass import WeierstrassForm
  sage: R.<x,y> = QQ[]
  sage: cubic = x^3 + y^3 + 1
  sage: WeierstrassForm(cubic)  # cubic in P^2
  (0, -27/4)
  sage: WeierstrassForm(x^4 + y^2 + 1)  # in P^2[112]
  (-4, 0)
  sage: WeierstrassForm(x^2*y^2 + x^2 + y^2 + 1)   # in P^1xP^1
  (-16/3, 128/27)

Apply

Attachments (5)

trac_13084_ppl_lattice_polygon.patch (42.9 KB) - added by vbraun 8 years ago.
Updated patch
trac_13084_toric_weierstrass.patch (38.7 KB) - added by vbraun 8 years ago.
Updated patch
trac_13084_ppl_cleanup-fc.patch (15.1 KB) - added by chapoton 8 years ago.
trac_13084_weierstrass_cleanup-fc.patch (17.0 KB) - added by chapoton 8 years ago.
trac_13084_misc.patch (3.2 KB) - added by vbraun 8 years ago.
Updated patch

Download all attachments as: .zip

Change History (33)

comment:1 Changed 9 years ago by vbraun

  • Cc novoselt added
  • Dependencies set to #12553
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by nbruin

Excellent work! In all these cases, the genus 1 curve is a cover of its jacobian in a relatively canonical way. With your approach, is it reasonably possible to compute these covering maps? Doing so gives a kind of "certificate". One starting point would be

T.A Fisher, The invariants of a genus one curve, Proc. Lond. Math. Soc., 97(3):753--782, 2008.

also on arXiv.

comment:3 Changed 9 years ago by vbraun

The reference looks interesting, I'll have a look!

Though I think there is already too much code in this ticket, any further features should go to a followup.

comment:4 Changed 9 years ago by vbraun

  • Dependencies changed from #12553 to #12553, #13451

I've split off the classical invariant theory stuff into a different module (#13451), this is much cleaner now. Still needs review ;-)

comment:5 Changed 8 years ago by chapoton

apply only trac_13084_toric_weierstrass.patch

comment:6 Changed 8 years ago by vbraun

  • Description modified (diff)

comment:7 Changed 8 years ago by vbraun

rediffed for sage-5.8.beta0

comment:8 follow-up: Changed 8 years ago by vbraun

  • Cc dimpase added

This is mostly geometry of polygons with a little bit of invariant theory thrown in - Dima, do you feel comfortable reviewing the ticket?

comment:9 in reply to: ↑ 8 Changed 8 years ago by dimpase

Replying to vbraun:

This is mostly geometry of polygons with a little bit of invariant theory thrown in - Dima, do you feel comfortable reviewing the ticket?

This is surely a fun ticket!

But I am not familiar with the number theory part of it, and lately falling behind with everything, including reviewing my son's nappy's :–) Besides, it ought to be reviewed by number theorists.

Let me in turn invite you to consider taking part in this: http://web.spms.ntu.edu.sg/~dima/IMS2013/

comment:10 Changed 8 years ago by vbraun

  • Cc mstreng added

For the record, there isn't really any number theory in here. Maybe Marco is interested in reviewing this? ;-)

comment:11 Changed 8 years ago by chapoton

  • Status changed from needs_review to needs_work
  • Work issues set to coverage, doctest

See the bot report:

  • coverage is not 100%
  • needs to use the new doctest continuation ...:

Changed 8 years ago by vbraun

Updated patch

Changed 8 years ago by vbraun

Updated patch

comment:12 Changed 8 years ago by vbraun

Fixed

comment:13 Changed 8 years ago by chapoton

  • Status changed from needs_work to needs_review

comment:14 Changed 8 years ago by vbraun

Frédéric, are you going to review this ticket?

comment:15 Changed 8 years ago by chapoton

I have added a "cosmetic cleanup" patch for the file weierstrass.py. In particular, it removes many unused import statements.

comment:16 Changed 8 years ago by vbraun

Thanks, looks good to me.

Changed 8 years ago by chapoton

comment:17 Changed 8 years ago by chapoton

  • Description modified (diff)

Another cosmetic-cleanup patch. Similar work on import statements and pep8

Changed 8 years ago by chapoton

comment:18 Changed 8 years ago by vbraun

Looks good to me.

I take it you forgot to set the ticket to positive review after posting patches for all of the new code?

comment:19 Changed 8 years ago by chapoton

well, I am not able to seriously review the math..

comment:20 follow-up: Changed 8 years ago by dimpase

Attempting to look at the maths side. It's confusing that you start off talking about a toric elliptic curve, but then speak about "the elliptic curve". What is going on there?

further nitpicks:

There are 16 reflexive polygons in 2-d.

Could you add a reference and change this to

There are 16 reflexive polygons in the plane.

And add an exact reference to the next statement.

Each defines a toric fano  variety, which (in 2-d) has a unique crepant resolution to a smooth 
 toric surface.

Which should also be changed to

Each of them defines a toric Fano  variety...

Then,

It turns out that the anticanonical hypersurface equation...

Reference?

comment:21 in reply to: ↑ 20 Changed 8 years ago by vbraun

Replying to dimpase:

And add an exact reference to the next statement.

Each defines a toric fano  variety, which (in 2-d) has a unique crepant resolution to a smooth 
 toric surface.

Thats just the fact that a fan in the plane has a unique refinement into smooth cones. CLS just say that "its easy to see" (p. 500)

comment:22 follow-up: Changed 8 years ago by vbraun

  • Description modified (diff)

Done

comment:23 in reply to: ↑ 22 Changed 8 years ago by dimpase

Replying to vbraun:

Done

capitalize fano. It's a name, after all!

comment:24 Changed 8 years ago by dimpase

  • Status changed from needs_review to positive_review

hope you will update the relevant patch to change fano to Fano in docstrings...

Changed 8 years ago by vbraun

Updated patch

comment:25 Changed 8 years ago by vbraun

Thanks, I fixed the "Fano".

Speaking of, its been grinding my gears the last couple of days that hilbert_series is lower case in Sage ;-)

comment:26 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.10 to sage-5.11
  • Reviewers set to Dmitrii Pasechnik
  • Work issues coverage, doctest deleted

comment:27 Changed 8 years ago by dimpase

  • Reviewers changed from Dmitrii Pasechnik to Frédéric Chapoton, Dmitrii Pasechnik

comment:28 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.11.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.