Implement Riemann theta functions

93749b3a-c0a4-47a7-b178-004ba08b0b97 commented 15 years ago

In the theory of differential equations and Abelian varieties, Riemann theta functions and their relatives play an important role. Implement these in Sage!

See also:

RiemannTheta -- A Sagemath package for evaluating Riemann theta functions with characteristics numerically to arbitrary precision, as well as their derivatives

CC: @cswiercz @fredrik-johansson @mstreng @jpflori @slel

Component: numerical

Keywords: riemann theta klein

Author: Nick Alexander, Chris Swierczewski

Branch/Commit: u/chapoton/6371 @ 0342ce9

Reviewer: Marco Streng, Bernard Deconinck

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

93749b3a-c0a4-47a7-b178-004ba08b0b97 commented 15 years ago

Attachment: trac_6371-riemann-theta.patch.gz

93749b3a-c0a4-47a7-b178-004ba08b0b97 commented 15 years ago

comment:1

I want to make sure this code doesn't get lost. It's very much [needs work] -- I don't think all tests are tagged optional and long correctly. I'm not certain that the output is correct to very high accuracy, either -- magma and mathematica disagree, and maple is too slow to evaluate to high precision.

Fredrik, somewhere in here there are lots of calls to the incomplete gamma function. I used low precision as much as possible as you will see, because I don't really need high precision, I just need large outputs.

Chris, this applies against 4.0.2.alpha1 at least. Documentation and testing appreciated!

93749b3a-c0a4-47a7-b178-004ba08b0b97 commented 15 years ago

comment:2

Marco, you might be interested in this work in progress code too.

fredrik-johansson commented 15 years ago

comment:3

Is double precision sufficient? Are the arguments large, integers or half-integers? In either case, very fast evaluation of the incomplete gamma function should be possible.

williamstein commented 15 years ago

comment:4

To use this, type

sage: hg_sage.apply('https://github.com/sagemath/sage-prod/files/10645258/trac_6371-riemann-theta.patch.gz')
sage: quit

sage -br

sage: now you have the new code

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:6

Over the past month or two I've done considerable work vetting out the bugs and errors in the above code. Within a week or so I hope to post a replacement patch. Hence, the change of owner. I'll keep everyone who's interested posted on my progress.

A word of warning, though: since I know a lot of people who are interested in my getting Riemann theta into Sage as soon as possible I had to strip Shimura phi, Siegel theta, and Klein P. I'm not qualified to review / bug fix that part of the code. I hope this is okay with the original authors and people who are interested in seeing this patch resolved. Perhaps we can move these implementations into a new patch once Riemann theta is implemented.

Again, I'll be posting a replacement patch sometime within the next two weeks or so.

93749b3a-c0a4-47a7-b178-004ba08b0b97 commented 13 years ago

comment:7

A word of warning, though: since I know a lot of people who are interested in my getting Riemann theta into Sage as soon as possible I had to strip Shimura phi, Siegel theta, and Klein P. I'm not qualified to review / bug fix that part of the code. I hope this is okay with the original authors and people who are interested in seeing this patch resolved.

I am the original author, and I think this is a very reasonable decision! You should know that I use "Siegel theta" and "Riemann theta" interchangeably. Riemann theta often seems to mean just with characteristics (0, 0, ..., 0) and Siegel theta seems to be a Mathematica-ism. Whatever we decide on is (probably) fine by me.

Perhaps we can move these implementations into a new patch once Riemann theta is implemented.

These bits are interesting for my research, but probably are not all that interesting more generally. Cut them for now.

Thanks for you efforts, Chris!

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:8

I am the original author, and I think this is a very reasonable decision! You should know that I use "Siegel theta" and "Riemann theta" interchangeably. Riemann theta often seems to mean just with characteristics (0, 0, ..., 0) and Siegel theta seems to be a Mathematica-ism. Whatever we decide on is (probably) fine by me. Thanks for you efforts, Chris!

Huzzah! Thank _you_ for your help!

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Attachment: trac_6371-riemann-theta-REPLACEMENT.patch.gz

Replacement for first patch by ncalexan

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Changed author from Nick Alexander to Nick Alexander, cswiercz

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:9

Note that the second patch is meant to replace the first. Many many changes were made!

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:10

Some clarifications and fine tuning needs to be made to the documentation.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Part 2 of the replacement patch. Includes documentation fixes.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:11

Attachment: trac_6371-riemann-theta-REPLACEMENT-PART2.patch.gz

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Attachment: trac_6371_riemann-theta-REPLACEMENT-PART3.patch.gz

Minor last-minute bug.

mstreng commented 13 years ago

Description changed:

--- 
+++ 
@@ -1 +1,12 @@
 In the theory of differential equations and abelian varieties, Riemann theta functions and there relatives play an important role.  Implement these in sage!
+
+Apply:
+
+* [attachment: trac_6371-riemann-theta-REPLACEMENT.patch](https://github.com/sagemath/sage-prod/files/10645259/trac_6371-riemann-theta-REPLACEMENT.patch.gz)
+
+* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART2.patch](https://github.com/sagemath/sage-prod/files/10645260/trac_6371-riemann-theta-REPLACEMENT-PART2.patch.gz)
+
+* [attachment: trac_6371_riemann-theta-REPLACEMENT-PART3.patch](https://github.com/sagemath/sage-prod/files/10645261/trac_6371_riemann-theta-REPLACEMENT-PART3.patch.gz)
+
+
+

mstreng commented 13 years ago

comment:12

(for patchbot)

Apply

mstreng commented 13 years ago

comment:13

Patchbot doesn't seem to get the message. New attempt: Apply trac_6371-riemann-theta-REPLACEMENT.patch, trac_6371-riemann-theta-REPLACEMENT-PART2.patch, trac_6371_riemann-theta-REPLACEMENT-PART3.patch

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:14

Replying to @mstreng:

Patchbot doesn't seem to get the message. New attempt: Apply trac_6371-riemann-theta-REPLACEMENT.patch, trac_6371-riemann-theta-REPLACEMENT-PART2.patch, trac_6371_riemann-theta-REPLACEMENT-PART3.patch

I tried just deleting the first patch called "trac_6371-riemann-theta.patch" but I don't have permissions to do so. I'm also unfamiliar with how to tell patchbot to ignore certain patches.

A manual importing of the three patches using sage -hg import <the patch> results in no issues.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Attachment: trac_6371-riemann-theta-REPLACEMENT-PART4.patch.gz

More documentation fixes.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Description changed:

--- 
+++ 
@@ -8,5 +8,7 @@

 * [attachment: trac_6371_riemann-theta-REPLACEMENT-PART3.patch](https://github.com/sagemath/sage-prod/files/10645261/trac_6371_riemann-theta-REPLACEMENT-PART3.patch.gz)

+* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART4.patch](https://github.com/sagemath/sage-prod/files/10645262/trac_6371-riemann-theta-REPLACEMENT-PART4.patch.gz)

+

mstreng commented 13 years ago

comment:16

doctest failures (details emailed to author)

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Fixed derivative calculation errors that resulted in incorrect doctests.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:17

Attachment: trac_6371-riemann-theta-REPLACEMENT-PART5.patch.gz

Fixed issues with computing derivatives that resulted in doctest failures. Minor documentation fixes as well.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

Description changed:

--- 
+++ 
@@ -10,5 +10,6 @@

 * [attachment: trac_6371-riemann-theta-REPLACEMENT-PART4.patch](https://github.com/sagemath/sage-prod/files/10645262/trac_6371-riemann-theta-REPLACEMENT-PART4.patch.gz)

+* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART5.patch](https://github.com/sagemath/sage-prod/files/10645263/trac_6371-riemann-theta-REPLACEMENT-PART5.patch.gz)

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 13 years ago

comment:19

With further examples, I noticed that there were some calculation issues. Traced this back to mixture of notation under Heil's transformation. Rewriting good chunk of the code using only the notation from "Computing Riemann Theta Functions" by Deconinck et. al.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Attachment: trac-6371.patch.gz

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Attachment: trac-6371-part2.patch.gz

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Description changed:

--- 
+++ 
@@ -2,14 +2,8 @@

 Apply:

-* [attachment: trac_6371-riemann-theta-REPLACEMENT.patch](https://github.com/sagemath/sage-prod/files/10645259/trac_6371-riemann-theta-REPLACEMENT.patch.gz)
+* [attachment: trac-6371.patch](https://github.com/sagemath/sage-prod/files/10645264/trac-6371.patch.gz)

-* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART2.patch](https://github.com/sagemath/sage-prod/files/10645260/trac_6371-riemann-theta-REPLACEMENT-PART2.patch.gz)
-
-* [attachment: trac_6371_riemann-theta-REPLACEMENT-PART3.patch](https://github.com/sagemath/sage-prod/files/10645261/trac_6371_riemann-theta-REPLACEMENT-PART3.patch.gz)
-
-* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART4.patch](https://github.com/sagemath/sage-prod/files/10645262/trac_6371-riemann-theta-REPLACEMENT-PART4.patch.gz)
-
-* [attachment: trac_6371-riemann-theta-REPLACEMENT-PART5.patch](https://github.com/sagemath/sage-prod/files/10645263/trac_6371-riemann-theta-REPLACEMENT-PART5.patch.gz)
+* [attachment: trac-6371-part2.patch](https://github.com/sagemath/sage-prod/files/10645265/trac-6371-part2.patch.gz)

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Attachment: trac-6371-part3.patch.gz

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Description changed:

--- 
+++ 
@@ -6,4 +6,6 @@

 * [attachment: trac-6371-part2.patch](https://github.com/sagemath/sage-prod/files/10645265/trac-6371-part2.patch.gz)

+* [attachment: trac-6371-part3.patch](https://github.com/sagemath/sage-prod/files/10645266/trac-6371-part3.patch.gz)

+

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Description changed:

mstreng commented 12 years ago

Reviewer: Marco Streng, bern4rd (Bernard Deconinck??)

mstreng commented 12 years ago

Changed author from Nick Alexander, cswiercz to Nick Alexander, Chris Swierczewski

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

Changed reviewer from Marco Streng, bern4rd (Bernard Deconinck??) to Marco Streng, Bernard Deconinck

jdemeyer commented 12 years ago

comment:27

I get a doctest failure in devel/sage/sage/all.py

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

comment:28

Replying to @jdemeyer:

I get a doctest failure in devel/sage/sage/all.py

I see. The error is:


sage -t  "devel/sage-6371/sage/all.py"                      
**********************************************************************
File "/home/cswiercz/sage-4.7.1/devel/sage-6371/sage/all.py", line 17:
    sage: len(frames)
Expected:
    11
Got:
    26
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_0
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/cswiercz/.sage//tmp/all_20664.py
     [3.4 s]

However, I'm not qualified to make a fix since I don't really understand the purpose of this test. Does anyone on cc: know what's going on in this file and why my Riemann Theta code would affect it?

williamstein commented 12 years ago

comment:29

Chris: Have you read #10570? That explains what this test is about and why it is important. Your code evidently gets imported when Sage starts up, and does something potentially leaky.

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

comment:30

Replying to @williamstein:

Chris: Have you read #10570? That explains what this test is about and why it is important. Your code evidently gets imported when Sage starts up, and does something potentially leaky.

Yes, I read this ticket but I'm not sure how to resolve the "potential leakiness".

One candidate for where this issue might be coming from is my use of a class constructor. I have some code in riemann_theta.py in the following form:

def RiemannTheta_Constructor(foo, bar):
    class RiemannTheta(Function_RiemannTheta):
        def __init__(...):
            self.foo = foo
            self.bar = bar
            Function_RiemannTheta.__init__()

    return RiemannTheta(...)

RiemannTheta = RiemannTheta_Constructor

class Function_RiemannTheta(BuiltinFunction):
   ...

I needed to do things this way so I could parameterize RiemannTheta by things such as directional derivative and Riemann matrix. Otherwise, things behave not as expected due to the way BuiltinFunction behaves. (For example, two instances of RiemannTheta would share the same Riemann matrix or derivatives.) With the above code structure I found that these issues don't arise and two instances of RiemannTheta behaved independently as expected.

I'd hate to introduce memory leaks due to my lack of understanding of BuiltinFunction. (Granted, the documentation for creating classes that inherit BuiltinFunction is severely lacking as well.) However, I don't see how I'd start fixing the issue aside from increasing len(frames) in the error message above from 11 to 26. Any suggested resources on how I could better perform what I'm doing in the class and function construction above? Again, the BuiltinFunction documentation wasn't helpful beyond copying what preexisting inheritors in sage.functions do.

mstreng commented 12 years ago

comment:31

I don't know much about BuiltinFunction, but I suspect that it isn't meant for what you are trying to do. All the examples that I know of BuiltinFunction are single fixed functions, independent of any parameters, such as cos, min, max, IndefiniteIntegral, Gamma, dirac_delta, etc.

Your objects however depend on a parameter Omega, and are functions only of z. They are maps theta(Omega): CC^g --> CC : z |--> theta(z). This will not be a "built-in function of Sage" for every Omega, so it does not sound to me like this should be a BuiltinFunction object. Maybe the memory leak is caused by this.

The function theta: CC^g x H_g --> CC : (z, Omega) |--> theta(z, Omega) definitely make sense as a built-in function, but not a function theta(Omega) : CC^g --> C that depends on Omega. In other words, if you have a function for which Omega is not a parameter but an actual variable, then that would make a good built-in function.

ps. Could you make sure you break the lines in the patch at 79 characters?

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

comment:32

Hello mstreng,

Replying to @mstreng:

I don't know much about BuiltinFunction, but I suspect that it isn't meant for what you are trying to do. All the examples that I know of BuiltinFunction are single fixed functions, independent of any parameters, such as cos, min, max, IndefiniteIntegral, Gamma, dirac_delta, etc.

It was my misunderstanding that BuiltinFunction is necessary for doing symbolic manipulation. My only reason for using it is so I can compute derivatives, etc. If you know of an alternate way to enable symbolics then I'd love to hear about it!

Your objects however depend on a parameter Omega, and are functions only of z. They are maps theta(Omega): CC^g --> CC : z |--> theta(z). This will not be a "built-in function of Sage" for every Omega, so it does not sound to me like this should be a BuiltinFunction object. Maybe the memory leak is caused by this.

I'll experiment with the code and see if this is an issue.

In practice, $\Omega$ is treated more as a parameter than an argument of the Riemann theta function. That is, $\Omega$ is usually fixed and $z$ varies. Several users of my code would like to see this behavior be reflected by having the Riemann theta function be "initialized" by this matrix.

However, if memory leaks are unavoidable in this context then I suppose I'll just have to make $\Omega$ behave like an argument no different from $z$.

The function theta: CC^g x H_g --> CC : (z, Omega) |--> theta(z, Omega) definitely make sense as a built-in function, but not a function theta(Omega) : CC^g --> C that depends on Omega. In other words, if you have a function for which Omega is not a parameter but an actual variable, then that would make a good built-in function.

Makes sense.

ps. Could you make sure you break the lines in the patch at 79 characters?

Of course! (As an in-terminal emacs user I usually try to do this anyway.)

mstreng commented 12 years ago

comment:33

Hi Chris,

Replying to @cswiercz:

If you know of an alternate way to enable symbolics then I'd love to hear about it!

I don't: I've never implemented any of them.

In practice, $\Omega$ is treated more as a parameter than an argument of the Riemann theta function. That is, $\Omega$ is usually fixed and $z$ varies.

:) Actually, when I use theta functions in my research, Omega is usually the variable and z is usually some fixed vector in with entries in QQ.

Anyway, what is the (programming technical) reason for having objects that depend on Omega? Is there some kind of caching involved? Some part of the algorithm that depends on Omega only and that you only want to do once?

Marco

3421e925-4b79-48cc-ba65-9123d4ab1b38 commented 12 years ago

comment:34

Marco,

:) Actually, when I use theta functions in my research, Omega is usually the variable and z is usually some fixed vector in with entries in QQ.

Anyway, what is the (programming technical) reason for having objects that depend on Omega? Is there some kind of caching involved? Some part of the algorithm that depends on Omega only and that you only want to do once?

Interesting. I've only seen one other instance in which Omega was varied: it was in the context of solving particular boundary value problems.) If you don't mind, do you have a paper you can send me about your work? (cswiercz [at] gmail ]dot[ com)

I agree that there should be no distinction between the two parameters. From what I understood about BuiltinFunction}}, though, the purpose of distinguishing Omega as a parameter is to make it easy to send symbolic output. Since I initially made this decision, however, I've learned more about the way Sage ({{{BuiltinFuncion?) handles inputs and determines if they're symbolic. A rewriting of the way arguments are sent to RiemannTheta should be possible.

I'll see what I can do about restructuring the code. Thank you very much for your input and suggestions. I'll keep everyone posted on my (slow) progress.

fchapoton commented 9 years ago

comment:39

I made a git branch from the patches, but it does not work. I have not tried to find where problems are.

New commits:

`403854b`	`Added Riemann theta functions to Sage.`
`ff08d10`	`Minor doctest issues resolved with riemann_theta.py.`
`3d0a00c`	`Fixed additional errors with docstrings and symmetric Riemann matrix testing.`
`afacea6`	`trac #6371 code cleanup`

fchapoton commented 9 years ago

Branch: u/chapoton/6371

fchapoton commented 9 years ago

Commit: afacea6

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

Changed commit from afacea6 to 5caa1ec

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

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

5caa1ec trac #6371 ref links

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

Changed commit from 5caa1ec to 696b994

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

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

`72a17d9`	`Merge branch 'u/chapoton/6371' into 7.1.b3`
`5e44969`	`trac #6371 partial correction, tolerance added`
`696b994`	`trac #6371 details`

sagemath / sage

Implement Riemann theta functions #6371