sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Cython wrapper for Hein's ternary Birch code #23342

Open kedlaya opened 7 years ago

kedlaya commented 7 years ago

One product of Jeffery Hein's PhD thesis is some impressive C++ code for computing Hecke operators on spaces of modular forms, based on a method of Birch involving reduction of ternary quadratic forms. Besides being very fast, the code produces sparse integer matrices for individual Atkin-Lehner eigenspaces.

The code is available at

https://github.com/jefferyphein/ternary-birch

and it would be great to get it into Sage. This might require some better packaging on the upstream end, but for now I'm focusing on building a working Cython wrapper. I have some progress on that, but I'm new to the Cython/C++ interaction and that is holding things up.

CC: @jvoight @tornaria

Component: modular forms

Keywords: Hecke operators, days88, sd91

Branch/Commit: public/23342 @ 13591fe

Issue created by migration from https://trac.sagemath.org/ticket/23342

jdemeyer commented 7 years ago

Replying to @kedlaya:

This might require some better packaging on the upstream end

I see that this package uses SCons unfortunately. I personally hate SCons because it is much less portable than other alternatives. We used to have a few packages in Sage using SCons, but we either got rid of them or we ported them to a different build system. Besides that, there is also the simple fact that SCons is not shipped with Sage. Luckily, the package is simple enough, so porting to autotools should not be too hard.

kedlaya commented 7 years ago
comment:2

Replying to @jdemeyer:

Replying to @kedlaya:

This might require some better packaging on the upstream end

I see that this package uses SCons unfortunately. I personally hate SCons because it is much less portable than other alternatives. We used to have a few packages in Sage using SCons, but we either got rid of them or we ported them to a different build system. Besides that, there is also the simple fact that SCons is not shipped with Sage. Luckily, the package is simple enough, so porting to autotools should not be too hard.

We may be able to get some help from upstream if it comes to that. Anyway, the purpose of this ticket is just the wrapper; I plan to open another ticket regarding packaging later if we get that far.

kedlaya commented 7 years ago

Attachment: wrapper.spyx.gz

Sage+Cython wrapper to Hein's C++ code

kedlaya commented 7 years ago
comment:3

I've attached a working wrapper to Hein's code. The code itself may be of some independent interest besides what I've exposed (e.g., for dealing with quaternion algebras), but I'm not sure if it can easily be made 32-bit-safe. (This has been an issue with incorporating other packages; see for instance #965.)

By the way, the publicly available reference for Birch's method is Birch's 1991 article "Hecke actions on classes of ternary quadratic forms".

kedlaya commented 7 years ago

Changed keywords from Hecke operators to Hecke operators, days88

kedlaya commented 7 years ago
comment:5

It seems that I managed to include enough build directives in my Cython that there is no need for Scons (just tested this on a fresh copy).

fchapoton commented 7 years ago

Commit: 890a13a

fchapoton commented 7 years ago

New commits:

890a13atrac 23342 Cython wrapper for Hein's ternary Birch code
fchapoton commented 7 years ago

Branch: public/23342

roed314 commented 7 years ago

Changed keywords from Hecke operators, days88 to Hecke operators, days88, sd91

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

6e5e0c1trac 23342 Cython wrapper for Hein's ternary Birch code
ea9a3dcUpdate pragmas to distutils, add doctests
60edbc6Merge branch 'public/23342' of git://trac.sagemath.org/sage into t/23342/public/23342
9c49140trac 23342 Cython wrapper for Hein's ternary Birch code
72f1205Merge branch 'temp-23342' into t/23342/public/23342
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Changed commit from 890a13a to 72f1205

kedlaya commented 6 years ago
comment:10

Quick update: in light of the deprecation warnings, I changed the pragmas to use distutils.

This currently "works" in the following sense: if I put hein_wrapper.pyx in the same folder as the Hein code, then running load("hein_wrapper.pyx") works in Sage 8.2. However, I haven't thought at all about packaging this to actually build as part of Sage.

jdemeyer commented 6 years ago
comment:11

I see two options:

  1. Simply "fork" the upstream into Sage, meaning copy the code.

  2. Add a proper build system to the upstream package (for example using autotools) and then add that to Sage as optional package.

kedlaya commented 6 years ago
comment:12

My impression is that Hein would be willing to accept code upstream (he is not working in academia and may not be spending too much time on this anymore). So setting up a proper build system upstream might be the best plan here; but I'm not competent to do this myself.

kedlaya commented 5 years ago

Changed commit from 72f1205 to 13591fe

kedlaya commented 5 years ago
comment:13

Update: Hein has released a 2.0 version of his code with a much better Sage wrapper than the one I wrote. So we might be able to get by with forking the upstream and discarding my previous code (which would be fine with me).


New commits:

13591fetrac 23342 doc formatting