Opened 5 years ago
Last modified 3 years ago
#23342 new enhancement
Cython wrapper for Hein's ternary Birch code
Reported by:  kedlaya  Owned by:  

Priority:  minor  Milestone:  sage8.0 
Component:  modular forms  Keywords:  Hecke operators, quadratic forms, days88, sd91 
Cc:  jvoight, tornaria  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/23342 (Commits, GitHub, GitLab)  Commit:  13591fe26d74f540cd5bd56489fc8b522ef7fd4a 
Dependencies:  Stopgaps: 
Description
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 AtkinLehner eigenspaces.
The code is available at
https://github.com/jefferyphein/ternarybirch
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.
Attachments (1)
Change History (14)
comment:1 in reply to: ↑ description ; followup: ↓ 2 Changed 5 years ago by
comment:2 in reply to: ↑ 1 Changed 5 years ago by
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.
comment:3 Changed 4 years ago by
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 32bitsafe. (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".
comment:4 Changed 4 years ago by
 Keywords days88 added
comment:5 Changed 4 years ago by
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).
comment:6 Changed 4 years ago by
 Cc tornaria added
comment:7 Changed 4 years ago by
 Branch set to public/23342
 Commit set to 890a13a748e171006f4f4a7b6d951d5251707e87
New commits:
890a13a  trac 23342 Cython wrapper for Hein's ternary Birch code

comment:8 Changed 4 years ago by
 Keywords sd91 added
comment:9 Changed 4 years ago by
 Commit changed from 890a13a748e171006f4f4a7b6d951d5251707e87 to 72f120587aefe941a33746173122d5efcafb7808
Branch pushed to git repo; I updated commit sha1. New commits:
6e5e0c1  trac 23342 Cython wrapper for Hein's ternary Birch code

ea9a3dc  Update pragmas to distutils, add doctests

60edbc6  Merge branch 'public/23342' of git://trac.sagemath.org/sage into t/23342/public/23342

9c49140  trac 23342 Cython wrapper for Hein's ternary Birch code

72f1205  Merge branch 'temp23342' into t/23342/public/23342

comment:10 Changed 4 years ago by
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.
comment:11 Changed 4 years ago by
I see two options:
 Simply "fork" the upstream into Sage, meaning copy the code.
 Add a proper build system to the upstream package (for example using autotools) and then add that to Sage as optional package.
comment:12 Changed 4 years ago by
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.
comment:13 Changed 3 years ago by
 Commit changed from 72f120587aefe941a33746173122d5efcafb7808 to 13591fe26d74f540cd5bd56489fc8b522ef7fd4a
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:
13591fe  trac 23342 doc formatting

Replying to kedlaya:
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.