search

sagemath / sage

Main repository of SageMath. Now open for Issues and Pull Requests.
https://www.sagemath.org
Other
1.08k stars 392 forks source link

Wrapping lcalc library #5396

Closed b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 closed 13 years ago

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 15 years ago

Wrapping lcalc library. This patch depends spkg upgrade in #4793

CC: @craigcitro @JohnCremona @sagetrac-bober mrubinst@math.uwaterloo.ca @sagetrac-mvngu

Component: number theory

Keywords: lcalc

Author: Rishikesh, Yann Laigle-Chapuy

Reviewer: John Cremona, David Kirkby, Alex Ghitza

Merged: sage-4.5.2.alpha1

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

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 15 years ago
comment:2

Hi Rishi,

No patch, not relevant for 3.4 ;)

Cheers,

Michael

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Attachment: Lcalc_in_sage.sws.gz

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:3

I am attaching a lcalc library wrapper and a sage worksheet showing how to use it. It needs the latest lcalc release. Please see http://www.math.uwaterloo.ca/~mrubinst

Copy all the include files from lcalc/include to $SAGE_ROOT/local/include . Copy the libLfunction.so from lcalc/src to $SAGE_ROOT/local/lib Then you can compile the wrapper.

The attached worksheet gives examples

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:4

I am writing some functions which will generate lcalc objects for Dirichlet L functions, Elliptic Curve L functions, Cuspform L functions, which I am putting in sage/lfunctions/lcalc.py. Please let me know if you want something specific. To see what is needed by lcalc please look at page 18 of the following paper.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:5

Replying to @rishikesha: I forgot the link to paper.

http://arxiv.org/abs/math/0412181

See p.18

JohnCremona commented 14 years ago
comment:10

I was asked to review this so took a look, but I have no idea what is going on here. Isn't lcalc already in Sage? Or are only some of the functions already in Sage?

What on earth does the instruction "Copy all the include files from lcalc/include to $SAGE_ROOT/local/include . Copy the libLfunction.so from lcalc/src to $SAGE_ROOT/local/lib Then you can compile the wrapper." mean? Should that not be done when Sage is built, so is not the right thing to do to change the existing lcalc spkg in Sage (lcalc-20080205.p2) to a new spkg? Then the reviewer should install this new experimental spkg, apply a patch, and proceed as normal.

I also do not find it helpful to attach a worksheet with examples, because I do not use the notebook and so it's just one more complication.

Because of the above I changed the tag from "needs review" to "needs work", since I do not think this is ready for review.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Attachment: L-1.23.spkg.gz

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:11

I have attached the spkg file.

The lcalc in sage uses the command line version of lcalc. The lcalc library lot richer than what the command line exposes. For example, you cannot find the zeros of a Dirichlet L function or Modular form L function without writing a lcalc file in appropriate format and calling the command line version. Using this patch, it is possible to forgo the lcalc file. In fact, in one of the example, I have written a function which given a primitive Dirichlet character, generates a Lcalc object, say L

#This code is only for example, It will not execute
chi=DirichletGroup(5)[1]
L =dirLFuncGenerator(chi)
L.value(2+3*I) #gives value
L.find_zeros_via_N(10) # finds the first 10 zeros of this L function. (ordered by distance from real axis)

I will repost the code in the sage notebook again

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:12

Here I am reproducing what the sage worksheet does.

sage: import sage.libs.lcalc.for_testing
sage: from sage.libs.lcalc.for_testing import *
sage: D=DirichletGroup(5)
sage: chi=D[1]
sage: chi.values()
[0, 1, zeta4, -zeta4, -1]
sage: L=dirLFuncGenerator(chi)
sage: L.find_zeros_via_N(5)

[-4.13290370521285,
 6.18357819545085,
 8.45722917442324,
 -9.44293112972852,
 -11.2828964415816]

#Following Code produces the lowest zeros of Dirichlet L functions of Cubic Characters with modulus up to 10
sage: for k in srange(3,10):
    for chi in DirichletGroup(k,CyclotomicField(3)):
        if chi.is_primitive() and chi.order()==3:
            L=dirLFuncGenerator(chi)
            print k, L.find_zeros_via_N(1)[0]
....:             
7 4.35640162473628
7 -4.35640162473628
9 -3.44409315514895
9 3.44409315514895

#Here we create a L function corresponding to Elliptic Curve and find its zeros
sage: E=EllipticCurve('11a')
sage: L=elLFuncGenerator(E)
sage: L.find_zeros_via_N(5)

[6.36261389471309,
 8.60353961929075,
 10.0355090971811,
 11.4512586103452,
 13.5686390571300]

#Another example related to Elliptic Curves

sage: E=EllipticCurve('37')
sage: L2=elLFuncGenerator(E)
sage: print L2.value(.5), L2.value(.5,derivative=1)
0 0.305999824716000 elliptic curve
JohnCremona commented 14 years ago
comment:13

Thanks for the explanation.

I successfully installed the new lcalc spkg; then I successfully applied the patch and rebuilt sage.

Now testing sage/libs/lcalc gives this:

sage -t  "devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx"
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 411:
    sage: L.value(.6)
Expected:
    0.274633355856348 - 6.59869267328213e-18*I
Got:
    0.274633355856341 - 6.59869267328188e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 413:
    sage: L.value(.6+1*I)
Expected:
    0.362258705721100 + 0.433888250620852*I
Got:
    0.362258705721102 + 0.433888250620855*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 450:
    sage: L.find_zeros(1,15,.1)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472771, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 491:
    sage: L.find_zeros_via_N(3)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472772, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 642:
    sage: L.value(.5)
Expected:
    0.763747880117299 + 0.216964767518864*I
Got:
    0.763747880117295 + 0.216964767518863*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 644:
    sage: L.value(.6+5*I)
Expected:
    0.702723260619684 - 1.10178575243940*I
Got:
    0.702723260619682 - 1.10178575243940*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 789:
    sage: lcalc_zeta.value(CC(.4,.5))
Expected:
    -0.450728958517113 - 0.780511403019070*I
Got:
    -0.450728958517121 - 0.780511403019071*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 852:
    sage: lcalc_zeta.find_zeros_via_N(3)
Expected:
    [14.1347251417347, 21.0220396387715, 25.0108575801457]
Got:
    [14.1347251417347, 21.0220396387716, 25.0108575801457]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 174:
    sage: L.value(.5)
Expected:
    0.231750947504015 + 5.75329642226134e-18*I
Got:
    0.231750947504012 + 5.75329642226126e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 176:
    sage: L.value(CC(.2,.4))
Expected:
    0.102558603193338 + 0.190840777924700*I
Got:
    0.102558603193336 + 0.190840777924703*I
**********************************************************************
7 items had failures:

-- that is all just numerical noise probably because I'm testing on a 32-bit system and you made the tests on a 64-bit system (is that right?) That is not serious (though will need to be fixed).

In the "testing" file, are the functions intended for inclusion in Sage? If so, they need doctesting (and the names are a little strange) and they should perhaps be made into member functions of the appropriate classes (e.g. ell_rational_field).

In the docstrings explaining all the parameters, I don't think it is enough to just refer to the lcacl documentation. These are Sage functions and the inputs need to be documented withing Sage (however tedious that might be!)

There are quite a few typos in the documentation.

Would it be possible to have a base class and have the various L-series classes derive from it? That might reduce the code a little and make the structure clearer.

You have some input parameters which I think would more natually be True/False instead of 1/0 as now.

This has clearly been a huge amount of work and will be very useful. The things I mentioned above are all minor, but still they should be considered, so I have left the tag as "needs work", but I think this is quite close to being includable.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
-I am wrapping lcalc library. It is working but still needs some work.
+Thanks John for your feedback.  I was looking at the patch again, I am indeed seeing the typos, and places where documentation is far from clear. 
+
+While I was writing, I did think of having a base class and have various L series derive from it, but it had taken me several weeks of manipulation just to get a compilable code out of cython, so I balked. I will go back and see how to reduce the clutter.
b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:14

Replying to @JohnCremona:

Thanks for the explanation.

I successfully installed the new lcalc spkg; then I successfully applied the patch and rebuilt sage.

Now testing sage/libs/lcalc gives this:

sage -t  "devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx"
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 411:
    sage: L.value(.6)
Expected:
    0.274633355856348 - 6.59869267328213e-18*I
Got:
    0.274633355856341 - 6.59869267328188e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 413:
    sage: L.value(.6+1*I)
Expected:
    0.362258705721100 + 0.433888250620852*I
Got:
    0.362258705721102 + 0.433888250620855*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 450:
    sage: L.find_zeros(1,15,.1)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472771, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 491:
    sage: L.find_zeros_via_N(3)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472772, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 642:
    sage: L.value(.5)
Expected:
    0.763747880117299 + 0.216964767518864*I
Got:
    0.763747880117295 + 0.216964767518863*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 644:
    sage: L.value(.6+5*I)
Expected:
    0.702723260619684 - 1.10178575243940*I
Got:
    0.702723260619682 - 1.10178575243940*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 789:
    sage: lcalc_zeta.value(CC(.4,.5))
Expected:
    -0.450728958517113 - 0.780511403019070*I
Got:
    -0.450728958517121 - 0.780511403019071*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 852:
    sage: lcalc_zeta.find_zeros_via_N(3)
Expected:
    [14.1347251417347, 21.0220396387715, 25.0108575801457]
Got:
    [14.1347251417347, 21.0220396387716, 25.0108575801457]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 174:
    sage: L.value(.5)
Expected:
    0.231750947504015 + 5.75329642226134e-18*I
Got:
    0.231750947504012 + 5.75329642226126e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 176:
    sage: L.value(CC(.2,.4))
Expected:
    0.102558603193338 + 0.190840777924700*I
Got:
    0.102558603193336 + 0.190840777924703*I
**********************************************************************
7 items had failures:

-- that is all just numerical noise probably because I'm testing on a 32-bit system and you made the tests on a 64-bit system (is that right?) That is not serious (though will need to be fixed).

In the "testing" file, are the functions intended for inclusion in Sage? If so, they need doctesting (and the names are a little strange) and they should perhaps be made into member functions of the appropriate classes (e.g. ell_rational_field).

In the docstrings explaining all the parameters, I don't think it is enough to just refer to the lcacl documentation. These are Sage functions and the inputs need to be documented withing Sage (however tedious that might be!)

There are quite a few typos in the documentation.

Would it be possible to have a base class and have the various L-series classes derive from it? That might reduce the code a little and make the structure clearer.

You have some input parameters which I think would more natually be True/False instead of 1/0 as now.

This has clearly been a huge amount of work and will be very useful. The things I mentioned above are all minor, but still they should be considered, so I have left the tag as "needs work", but I think this is quite close to being includable.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:15

Replying to @rishikesha:

Thanks John for your feedback. I was looking at the patch again, I am indeed seeing the typos, and places where documentation is far from clear.

While I was writing, I did think of having a base class and have various L series derive from it, but it had taken me several weeks of manipulation just to get a compilable code out of cython, so I balked. I will go back and see how to reduce the clutter.

Rishi

Replying to @JohnCremona:

Thanks for the explanation.

I successfully installed the new lcalc spkg; then I successfully applied the patch and rebuilt sage.

Now testing sage/libs/lcalc gives this:

sage -t  "devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx"
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 411:
    sage: L.value(.6)
Expected:
    0.274633355856348 - 6.59869267328213e-18*I
Got:
    0.274633355856341 - 6.59869267328188e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 413:
    sage: L.value(.6+1*I)
Expected:
    0.362258705721100 + 0.433888250620852*I
Got:
    0.362258705721102 + 0.433888250620855*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 450:
    sage: L.find_zeros(1,15,.1)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472771, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 491:
    sage: L.find_zeros_via_N(3)
Expected:
    [6.64845334472771, 9.83144443288668, 11.9588456260835]
Got:
    [6.64845334472772, 9.83144443288667, 11.9588456260835]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 642:
    sage: L.value(.5)
Expected:
    0.763747880117299 + 0.216964767518864*I
Got:
    0.763747880117295 + 0.216964767518863*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 644:
    sage: L.value(.6+5*I)
Expected:
    0.702723260619684 - 1.10178575243940*I
Got:
    0.702723260619682 - 1.10178575243940*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 789:
    sage: lcalc_zeta.value(CC(.4,.5))
Expected:
    -0.450728958517113 - 0.780511403019070*I
Got:
    -0.450728958517121 - 0.780511403019071*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 852:
    sage: lcalc_zeta.find_zeros_via_N(3)
Expected:
    [14.1347251417347, 21.0220396387715, 25.0108575801457]
Got:
    [14.1347251417347, 21.0220396387716, 25.0108575801457]
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 174:
    sage: L.value(.5)
Expected:
    0.231750947504015 + 5.75329642226134e-18*I
Got:
    0.231750947504012 + 5.75329642226126e-18*I
**********************************************************************
File "/home/john/sage-4.1.1/devel/sage-tests/sage/libs/lcalc/lcalc_Lfunction.pyx", line 176:
    sage: L.value(CC(.2,.4))
Expected:
    0.102558603193338 + 0.190840777924700*I
Got:
    0.102558603193336 + 0.190840777924703*I
**********************************************************************
7 items had failures:

-- that is all just numerical noise probably because I'm testing on a 32-bit system and you made the tests on a 64-bit system (is that right?) That is not serious (though will need to be fixed).

In the "testing" file, are the functions intended for inclusion in Sage? If so, they need doctesting (and the names are a little strange) and they should perhaps be made into member functions of the appropriate classes (e.g. ell_rational_field).

In the docstrings explaining all the parameters, I don't think it is enough to just refer to the lcacl documentation. These are Sage functions and the inputs need to be documented withing Sage (however tedious that might be!)

There are quite a few typos in the documentation.

Would it be possible to have a base class and have the various L-series classes derive from it? That might reduce the code a little and make the structure clearer.

You have some input parameters which I think would more natually be True/False instead of 1/0 as now.

This has clearly been a huge amount of work and will be very useful. The things I mentioned above are all minor, but still they should be considered, so I have left the tag as "needs work", but I think this is quite close to being includable.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-Thanks John for your feedback.  I was looking at the patch again, I am indeed seeing the typos, and places where documentation is far from clear. 
-
-While I was writing, I did think of having a base class and have various L series derive from it, but it had taken me several weeks of manipulation just to get a compilable code out of cython, so I balked. I will go back and see how to reduce the clutter.
+Wrapping lcalc library
b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

basic lcalc wrapper

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:16

Attachment: lcalc_wrapper.patch.gz

The new patch adds lot of documentation, and clean few typos. There are three functions where True or False is more natural as parameter, and I am going to change it. That is pretty much the only change left. Any program written using this patch will continue to work when I make those changes, so this patch can still be included. I will probably post the patch by tomorrow. Today I received an email that someone wants to use this program so I am uploading this patch.

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:17

Replying to @rishikesha:

The new patch adds lot of documentation, and clean few typos.

With the patch lcalc_wrapper.patch, you create a new empty file called sage/libs/lcalc/__init__.py. It's dangerous to create empty files in the Sage library. Such files are known to result in missing files (hence corrupt the "main" repository) when creating a new source distribution of Sage. A solution is to put a comment in sage/libs/lcalc/__init__.py. The comment can just be something like:

# A comment here so this file is picked up when creating a new source distribution.
7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:18

I assume L-1.23.spkg is version 1.23 of lcalc. Then the package must use the name lcalc-1.23.spkg; the current version in Sage 4.1.1 is lcalc-20080205.p3.spkg. A name for the lcalc package like L-1.23.spkg can result in errors building or upgrading Sage. If this ticket is closed, then ticket #4793 should be closed as "wontfix".

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Attachment: minor_revision.patch.gz

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago

Attachment: make_init_nonempty.patch.gz

standalone patch, based on 4.3.alpha0

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:19

Attachment: trac_5396-refactor.patch.gz

I hope you won't mind, but I refactored a little bit your code to address reviewers comments.

In the provided patch:

After spkg install, only my patch needs to be applied.

Please let me know your opinion about all these.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:20

Replying to @sagetrac-ylchapuy:

Wonderfull!! I am looking at the code which has changed a lot, and it is impressive. I had written this as the lowest layer of a three layered interface to lcalc library. It was dirty, but it has taken me so long to add the necessary patches to lcalc itself, and then getting this thing to work with sage, that I balked when my attempts to derive the L-functions from a base class failed.

I am still absorbing the changes. I will write more later.

Rishi

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:21

Any move on this?

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:22

I apologise. I was busy with a gap which had popped up in my thesis. I will write by Sunday.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:23

Replying to @sagetrac-ylchapuy:

Any move on this?

I had a closer look, and I like what I see. The code is beautiful. I would like it to be included as it is. It does address everything which John Cremona had raised.

The last 2 functions which generate L functions for Dirichlet characters and Elliptic Curves were supposed to be there for testing and as example. For the time being I feel we can leave it like this till an improved version comes.

Craig: Can I give it a positive review? I am not sure how the reviewing process works in this case.

Rishi

JohnCremona commented 14 years ago
comment:24

Now there is code here from both rishi and ylchapuy it would be possible for each to review the other's code (since they both now know more about it than anyone else!) but it would be even better to have a third party do the job. I volunteer to do that, in the next day or so.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:25

Thanks John.

Replying to @JohnCremona:

Now there is code here from both rishi and ylchapuy it would be possible for each to review the other's code (since they both now know more about it than anyone else!) but it would be even better to have a third party do the job. I volunteer to do that, in the next day or so.

JohnCremona commented 14 years ago
comment:26

Here's what I did: made a fresh clone of 4.3.rc0, installed the spkg (sage -i lcalc-1.23.spkg), applied the latch, rebuilt. Then sage -t gives this:

john@ubuntu%sage -t sage-4.3.rc0/devel/sage-lcalc/sage/libs/lcalc/ 
sage -t  "devel/sage-lcalc/sage/libs/lcalc/__init__.py"     
     [0.1 s]
sage -t  "devel/sage-lcalc/sage/libs/lcalc/lcalc_Lfunction.pyx"
Exception NotImplementedError in 'sage.libs.lcalc.lcalc_Lfunction.Lfunction.__init_fun' ignored
-----------------------------------------------

Name of L_function: 
number of dirichlet coefficients = 5
coefficients are periodic
b[1] = 1
b[2] = -1
b[3] = -1
b[4] = 1
b[5] = 0

Q = 1.26156626101
OMEGA = (1,4.96506830649e-17)
a = 1 (the quasi degree)
gamma[1] =0.5    lambda[1] =(0,0)

number of poles (of the completed L function) = 0
-----------------------------------------------

-----------------------------------------------

Name of L_function: 
number of dirichlet coefficients = 5
coefficients are periodic
b[1] = (1,0)
b[2] = (6.12323399574e-17,1)
b[3] = (-6.12323399574e-17,-1)
b[4] = (-1,0)
b[5] = (0,0)

Q = 1.26156626101
OMEGA = (0.850650808352,0.525731112119)
a = 1 (the quasi degree)
gamma[1] =0.5    lambda[1] =(0.5,0)

number of poles (of the completed L function) = 0
-----------------------------------------------

**********************************************************************
File "/home/john/sage-4.3.rc0/devel/sage-lcalc/sage/libs/lcalc/lcalc_Lfunction.pyx", line 780:
    sage: L.value(0.5, derivative=1)
Expected:
    0.305999824716...
Got:
    0.305999723948232
-----------------------------------------------

Name of L_function: 
number of dirichlet coefficients = 5
coefficients are periodic
b[1] = 1
b[2] = -1
b[3] = -1
b[4] = 1
b[5] = 0

Q = 1.26156626101
OMEGA = (1,4.96506830649e-17)
a = 1 (the quasi degree)
gamma[1] =0.5    lambda[1] =(0,0)

number of poles (of the completed L function) = 0
-----------------------------------------------

**********************************************************************
1 items had failures:
   1 of   7 in __main__.example_24
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/john/.sage//tmp/.doctest_lcalc_Lfunction.py
     [2.2 s]

----------------------------------------------------------------------
The following tests failed:

    sage -t  "devel/sage-lcalc/sage/libs/lcalc/lcalc_Lfunction.pyx"
Total time for all tests: 2.2 seconds

I cannot really tell what is going on here, sorry.

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:27

The only lines really corresponding to the failure are:

File "/home/john/sage-4.3.rc0/devel/sage-lcalc/sage/libs/lcalc/lcalc_Lfunction.pyx", line 780:
    sage: L.value(0.5, derivative=1)
Expected:
    0.305999824716...
Got:
    0.305999723948232

The other ones comes from the doctesting of the _print_data_to_standard_output functions. I don't know how to deal with standard output easily in this case.

The problem is thus a numerical one. I don't know if it's normal to get such a difference in the result though. Should we just test for 0.305999... ?

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:28

Oh, I just noticed the Exception at the beginning. I don't know where it comes from.

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:29

Ok, the (ignored) Exception comes from the very first doctest where I try to create an instance of the Lfunction class which is intended to be virtual.

The attached patch correct this, reals with the numerical noise (should we?) as stated above, and also correct a small typo.

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:30

Attachment: trac_5396-review.patch.gz

JohnCremona commented 14 years ago
comment:31

Replying to @sagetrac-ylchapuy: Not sure when I'll get back to this -- family calls, and it might be a few days. It's a pity if the numerical accuracy depends so much on platform though.

2c00b582-cfe9-43b9-8124-fec470060704 commented 14 years ago
comment:32

Replying to @JohnCremona:

Not sure when I'll get back to this -- family calls, and it might be a few days.

No problem of course; Happy Christmas!

JohnCremona commented 14 years ago

Reviewer: John Cremona

JohnCremona commented 14 years ago
comment:33

OK, so I found a few minutes. New extra patch looks fine to me. Successfully applied and tested on 32 and 64 bit machines.

I guess that we'll next want to change, or add to, the interface to command-line lcalc in sage/lfunctions/lcalc.py -- but that's for another ticket.

JohnCremona commented 14 years ago

Author: rishi, ylchapuy

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:34

I did not check my email in the last 24 hours. Yes the derivatives will not be accurate. In fact it gets progressively wrong with higher derivatives. I will add a patch later to add to documentation to this fact. The derivative is calculated by calculating L function at two points very close and using the definition of derivative. It is not accurate at all.

The following error message is from lcalc library. It is debugging info which Mike uses when things go wrong.

----------------------------------------------

Name of L_function: 
number of dirichlet coefficients = 5
coefficients are periodic
b[1] = 1
b[2] = -1
b[3] = -1
b[4] = 1
b[5] = 0

Q = 1.26156626101
OMEGA = (1,4.96506830649e-17)
a = 1 (the quasi degree)
gamma[1] =0.5    lambda[1] =(0,0)

number of poles (of the completed L function) = 0
-----------------------------------------------
b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:35

I was wrong in the previous message. The print_data_to_standard_output is used for debugging in developing lcalc, and I used it to debug the wrapper initially when things were going totally awry. Although I rewrote few functions in lcalc to return values instead of printing in standard output, this function still prints everything out in standard output.

JohnCremona commented 14 years ago
comment:36

NB See also #7788 for some minor docstring changes which should be merged when this is.

mwhansen commented 14 years ago
comment:37

What all do I need apply for this? I get the following failure:

cd "/virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/devel/sage" && hg qser
cd "/virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/devel/sage" && hg qimport /home/mhansen/.sage/temp/boxen/29343/release_0/trac_5396-refactor.patch
cd "/virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/devel/sage" && hg qpush
cd "/virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/devel/sage" && hg qimport /home/mhansen/.sage/temp/boxen/29343/release_0/trac_5396-review.patch
cd "/virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/devel/sage" && hg qpush

=========================
 >>> Rebuilding Sage <<< 
=========================

cc1plus: warning: command line option "-Wstrict-prototypes" is valid for Ada/C/ObjC but not for C++
In file included from sage/libs/lcalc/lcalc_Lfunction.cpp:149:
sage/libs/lcalc/lcalc_sage.h:1:15: error: L.h: No such file or directory
In file included from sage/libs/lcalc/lcalc_Lfunction.cpp:149:
sage/libs/lcalc/lcalc_sage.h:27: error: expected constructor, destructor, or type conversion before ‘*’ token
sage/libs/lcalc/lcalc_sage.h:33: error: variable or field ‘del_Complexes’ declared void
sage/libs/lcalc/lcalc_sage.h:33: error: ‘Complex’ was not declared in this scope
sage/libs/lcalc/lcalc_sage.h:33: error: ‘A’ was not declared in this scope
sage/libs/lcalc/lcalc_sage.h:38: error: ‘Complex’ does not name a type
sage/libs/lcalc/lcalc_sage.h:44: error: variable or field ‘testL’ declared void
sage/libs/lcalc/lcalc_sage.h:44: error: ‘L_function’ was not declared in this scope
sage/libs/lcalc/lcalc_sage.h:44: error: ‘Complex’ was not declared in this scope
sage/libs/lcalc/lcalc_sage.h:44: error: ‘L’ was not declared in this scope
In file included from /usr/include/c++/4.2/ios:44,
                 from /usr/include/c++/4.2/ostream:45,
                 from /usr/include/c++/4.2/iostream:45,
                 from /virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/local//include/NTL/tools.h:14,
                 from /virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/local//include/NTL/ZZ.h:19,
                 from /virtual/scratch/mhansen/release/4.3.1/alpha0/sage-4.3.1.alpha0/local//include/csage/ntl_wrap.h:2,
                 from sage/libs/lcalc/lcalc_Lfunction.cpp:151:
/usr/include/c++/4.2/exception:40: error: expected declaration before end of line
b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:38

Replying to @mwhansen:

What all do I need apply for this? I get the following failure:

Are you using the spkg in this ticket?

89c6e537-b2e3-45e6-882d-d4957b74ffe5 commented 14 years ago
comment:39

After building lcalc-1.23.spkg and applying trac_5396-refactor.patch and trac_5396-review.patch, I get the following failures:

        sage -t -long devel/sage-main/sage/plot/line.py # 1 doctests failed
        sage -t -long devel/sage-main/sage/schemes/elliptic_curves/lseries_ell.py # 6 doctests failed
        sage -t -long devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py # 3 doctests failed
        sage -t -long devel/sage-main/sage/lfunctions/lcalc.py # 3 doctests failed

For example:

sage -t -long sage/plot/line.py
**********************************************************************
File "/virtual/scratch/rlm/sage-4.3.1.alpha2/devel/sage-main/sage/plot/line.py", line 375:
    sage: vals = E.lseries().values_along_line(1-I, 1+10*I, 100) # critical line
Expected nothing
Got:
    lcalc:  You need to uncomment the line: PARI_DEFINE = -DINCLUDE_PARI
    lcalc:  in the Makefile and do: 'make clean', then 'make' if you wish to use
    lcalc:  elliptic curve L-functions. Requires that you already have pari installed
    lcalc:  on your machine.
**********************************************************************
JohnCremona commented 14 years ago
comment:40

This suggests to me that the build script in the spkg needs attention.

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago

Attachment: lcalc-1.23.spkg.gz

b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:41

I have uncommented the appropriate line in the Makefile. lcalc uses pari to generate the dirichlet coefficients. If we use the wrapper, we can use sage to generate the dirichlet coefficients and feed it to lcalc.

JohnCremona commented 14 years ago
comment:42

rishi, I could not install this version of lcalc. I'll paste in below the error message I got. Before that, two comments: (1) the spkg should contain more than it does (an install script and the src directory). See http://www.sagemath.org/doc/developer/producing_spkgs.html: there should be a file SPKG.txt, and there should be mercurial files, and an check script.... (2) When you make new versions of the spkg it's helpful for the name to change sp people can tell them apart: here you could have called the first version lcalc-1.23.spkg, then lcalc-1.23.p1.spkg, etc. The changes in each version should be docuemtned in SPKG.txt.

Here's the error I got installing it:

...
g++ -Wa,-W -O3  -Wno-deprecated  -ffast-math -fPIC  -I../include -I/usr/local/include/pari  -DINCLUDE_PARI -c Lcommandline_elliptic.cc
Lcommandline_elliptic.cc: In function ‘void data_E(char*, char*, char*, char*, char*, int, Double*)’:
Lcommandline_elliptic.cc:124: error: ‘lgeti’ was not declared in this scope
make[1]: *** [Lcommandline_elliptic.o] Error 1
make[1]: Leaving directory `/home/john/sage-4.3.1.alpha1/spkg/build/lcalc-1.23/src/src'
make: *** [all] Error 2
Error building lcalc
cp: cannot stat `lcalc': No such file or directory

real    0m17.528s
user    0m16.449s
sys 0m0.564s
sage: An error occurred while installing lcalc-1.23
Please email sage-devel http://groups.google.com/group/sage-devel
explaining the problem and send the relevant part of
of /home/john/sage-4.3.1.alpha1/install.log.  Describe your computer, operating system, etc.
If you want to try to fix the problem yourself, *don't* just cd to
/home/john/sage-4.3.1.alpha1/spkg/build/lcalc-1.23 and type 'make check' or whatever is appropriate.
Instead, the following commands setup all environment variables
correctly and load a subshell for you to debug the error:
(cd '/home/john/sage-4.3.1.alpha1/spkg/build/lcalc-1.23' && '/home/john/sage-4.3.1.alpha1/sage' -sh)
When you are done debugging, you can type "exit" to leave the
subshell.
62e4eb80-de3a-49e1-9cca-aae48333ddc1 commented 14 years ago
comment:43

This problem has to do with the makefile for lcalc, which needs work. Note that it includes the following, which explains why it didn't compile after that INCLUDE_PARI line was uncommented.

ifeq ($(PARI_DEFINE),-DINCLUDE_PARI)
    #location of pari.h.
    LOCATION_PARI_H = /usr/local/include/pari #usual location

    #location of libpari.a or of libpari.so
    #depending on whether static or dynamic libraries are being used.
    #On mac os x it's the former, on linux I think usually the latter.
    LOCATION_PARI_LIBRARY = /usr/local/lib #usual location
else
    #supplied as a dummy so as to avoid more ifeq's below
    LOCATION_PARI_H = .
    LOCATION_PARI_LIBRARY = .
endif
62e4eb80-de3a-49e1-9cca-aae48333ddc1 commented 14 years ago

Suggestion: in the function Lfunction_from_character, instead of calling chi.gauss_sum(), call chi.gauss_sum_numerical(). It is much faster, and in the end we are only interested in the numerical approximation anyway.

sage: G = DirichletGroup(101)            
sage: chi = G[1]
sage: time A = chi.gauss_sum()
CPU times: user 19.05 s, sys: 0.02 s, total: 19.07 s
Wall time: 19.07 s
sage: time B = chi.gauss_sum_numerical()
CPU times: user 0.06 s, sys: 0.00 s, total: 0.06 s
Wall time: 0.06 s
b12b007b-ced8-4aef-b4e9-5b123b0bb8b3 commented 14 years ago
comment:45

Thanks Bober for the comment about compiling lcalc.

I wrote the two functions to generate L functions of Dirichlet characters and Elliptic Curves exclusively for testing of wrapper. It was not supposed to be for end users at all. I wrote very simple function whose sole purpose was to not introduce any bugs. In fact they were in a file called for_testing.py till the refactor of code.

Can we do optimization outside the wrapper itself.

Maybe we can write new functions in the lfunctions directory to generate elliptic curve, modular forms and dirichlet lfunctions.

Replying to @sagetrac-bober:

Suggestion: in the function Lfunction_from_character, instead of calling chi.gauss_sum(), call chi.gauss_sum_numerical(). It is much faster, and in the end we are only interested in the numerical approximation anyway.

sage: G = DirichletGroup(101)            
sage: chi = G[1]
sage: time A = chi.gauss_sum()
CPU times: user 19.05 s, sys: 0.02 s, total: 19.07 s
Wall time: 19.07 s
sage: time B = chi.gauss_sum_numerical()
CPU times: user 0.06 s, sys: 0.00 s, total: 0.06 s
Wall time: 0.06 s