New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade numpy to 1.5.1 #10792
Comments
comment:1
Updating should be straightforward but some doctest will break because of some extra verbosity:
and
I think that's all the failures related to numpy-1.5.1 |
comment:2
It sounds like there may be just a small change we should make to the Riemann map function. Do you know line it is complaining about? |
comment:3
I am not sure. I don't understand the code very well in Riemann map. The only thing suspect to me is this
on line 499 in riemann.pyx. pt1 type has never been defined so I don't know if cython automatically give it the type of the right hand side or if it defaults to something else. In which case it would have to be cast. |
comment:4
Okay. I'll try to do some printf debugging to see if I can find it "soon". |
comment:5
Do you have a 1.5.1 spkg that you could post? It sounds like you already made one. |
comment:6
Replying to @jasongrout:
I get that a lot. I will prepare one soon. But I don't need a new spkg to test things in sage-on-gentoo because I can use the system version, which is how I am aware of this issue and a few others regarding upgrading components. |
comment:7
You can find a numpy-1.5.1 spkg here |
comment:8
I don't understand the riemann code very well either, but it seems like the warning is coming from the line where temp (a real array?) is being assigned a value from self.cps (a complex array)---see the code that is deleted in the trac-10792-riemann.patch file. I simplified the code a lot to avoid having a temporary array, but this changed lots of numerical results. So one question is: are the results it returns now better or worse than before? I also seem to get a huge number of |
comment:10
The warnings have been there since cython-0.13 has landed. The code itself is numerically sensitive to various details including lapack implementation. I will take a few slow days because the town where I live (Christchurch, New Zealand) has been hit by a 6.3 earthquake. Where I live is ok, where I work isn't. |
comment:11
CCing Ethan, since he was the original author on the riemann.pyx file. Ethan: I've been looking at the code that seems to cause two problems. First is a ComplexWarnings that now appears in numpy 1.5.1, which I believe comes from a temporary array that was assumed to be real, but maybe should be complex. I've rewritten the affected lines to not use a temporary array. The other problem seems like it is a problem even before the upgrade---I keep getting
I noticed several things that could be pulled out of the inner loop, and so rewrote this line in the attached patch (which I'll update shortly). However, I don't know this code or the algorithms very well---can you look at the patch? It appears that the warnings happen when there is a division by zero, as in this test case:
So should we turn off the error checking for this error when doing this operation since at least one entry in cp-cp[t] will be zero? Or should we check for this entry and not calculate it? |
comment:12
Attachment: trac-10792-riemann-complexwarnings.patch.gz Ethan, I've split the issue into two problems: the ComplexWarnings problem that I hopefully fixed with the patch on this ticket and the invalid division warning, which I've made #10821. Can you check this code and the numerical differences in output? I now get these failures:
It looks like the errors are all slight numerical errors. Can someone confirm that? |
This comment has been minimized.
This comment has been minimized.
comment:15
Sorry for the slow response. It looks like everything is slight numerical problems, which is expected given (as mentioned above) the numerical sensitivity. I'll happily test, but I'm not sure how to upgrade Sage's numpy. If someone can tell me how to get that running, I'll be able to confirm this patch and #10821. Side Note 1: I see that my code is apparently quite incomprehensible to people. The underlying mathematics is quite a bear, so it'll never be nice and accessible, but is there anything I can do to improve the documentation? Side Note 2: What's the status of fast_callable these days? Patch #8867 is still out there, and if fast_callable is ready, I think we might be able to merge that as well (otherwise, I can try amending it to work with fast_callable as is) |
comment:16
Replying to @sagetrac-evanandel:
You can go through the following steps:
could you refer to a review of it in a journal or an online resource?
No idea. |
This comment has been minimized.
This comment has been minimized.
comment:17
As for fast-callable, #5572 is still on my todo list, but isn't likely to get done in the near future (e.g., this semester). If someone else wants to work on it, they are more than welcome. |
comment:18
Replying to @jasongrout:
This is where it would be helpful doctests actually documented why the particular value is correct. I've seen sooooo many doctests where the "expected value" is whatever someone got on their computer and is not substantiated in any way as a comment in the code. Also, if the algorithm, or its implementation in Sage is has poor numerical stability, this should be documented. Could this be computed with Mathematica or Wolfram|Alpha to arbitrary precision? Just as thought. If so, that could be documented - we have permission from Wolfram Research to use Wolfram|Alpha for the purpose of comparing results and documenting those compassions. |
comment:19
Replying to @sagetrac-drkirkby:
Unfortunately to my knowledge, this is the only extant tool that performs this sort of Riemann map. I believe that there are one or two cases where the analytic map is known, so I can probably add some tests that check accuracy against that.
As far as I've seen, it's not unstable in the sense of dramatically losing accuracy, but the many numerical calculations are sensitive to slight differences in machine-level implementation. This results in slight differences in the final error. I should be able to do some error analysis and see if these deviations are within the bounds of the algorithm.
Not without complete reimplementation, and I know of no reason why their performance should be better than ours. You can increase the numerical precision of the computation by increasing N (the number of collocation points on the boundary.) I'll can create a couple of comparision tests that can be run on different machines to see if that decreases the numerical deviation. |
comment:20
Could someone run a test where after upgrading numpy they rebuild scipy before doing sage -b. Technically rebuilding scipy can be done after sage -b as it is a pure runtime dependency. Then test sage/matrix/matrix_double_dense.py to see if there are more warnings in there. I have one appearing after updating to scipy-0.9 and I just want to check if it wasn't hiding before. I am fairly sure it isn't but it has to be checked and I am still quite stretched resource wise after the earthquake. |
comment:22
See also #7831 for a fix for FreeBSD that perhaps can be part of this spkg, assuming we can find people to test on FreeBSD. |
comment:23
Replying to @kcrisman:
Sure, I'll have a look at incorporating it. Did you have a go at the patch in the present ticket? I am not impressed by the results but it could be a fluck. |
comment:24
Replying to @kiwifb:
No, I haven't had a chance to do this, and I won't be at the machine I use for testing this sort of thing (the older PPC Mac) for several days. Just wanting to point it out, in case it makes things easy. |
Attachment: trac_10792-fix_riemann_doctest.patch.gz fix the doctests in calculus/riemann.pyx |
comment:31
OK I attached a patch to adjust the riemann.pyx doctests. Since some of these tests are very sensitive I reduced the precision to which the results are tested. I also preserved any "e-xx" in the results so we know they are meant to be small values close to zero. If they move again it should be easier to figure if we just have changed the value of the small number that in a perfect world would go to zero. For integration I like to use quadratures personally. We could get them from gsl. It would depends of the number of sample points. Of course for any methods you choose you can find pitfalls with a given function. |
Changed author from Jason Grout to François Bissey, Jason Grout |
Changed reviewer from David Kirkby to David Kirkby, Karl-DIeter Crisman, Ethan Van Andel |
This comment has been minimized.
This comment has been minimized.
Changed work issues from doctest changes for patch to none |
comment:34
This fixes it for me, and in general this is good teamwork. I'm running long doctests now, but don't anticipate trouble, given that so many others have tested this already except for this patch which clearly affects only one file. Tomorrow I will try to test this on a PPC machine, which could conceivably lead to some noise issues or weirdness, but I know Francois has access to such a machine as well and probably wouldn't have suggested upgrading if that was a problem, so for now it's 'positive review'. |
comment:35
Replying to @kcrisman:
I don't have my iMac G4 anymore. It is the property of Massey University and I had to leave it behind when I left for the university of Canterbury in December. On the other hand I have access to IBM power 5 hardware now (running both linux and aix - to be upgraded to power 7 this year). But I haven't managed to get sage running on that yet. The linux side is running suse-9 (SLES) and is a right old mess. I probably shouldn't bother fixing the toolchain before the new gear arrives with fresh software. On the aix side I have to sort out compilers since CC default to XLC and fortran seems to default to GNU (never mind the fact XL fortran is present as well) and it stops at prereq. On the bright side gentoo prefix is "supported" on aix - so I may get somewhere that way :) |
comment:36
Okay, that makes my machine more valuable then. I'll definitely do my best to test this tomorrow, though of course it's extremely slow. But since the patches specific to these things are now in upstream, I feel confident.
Wow, Dave Kirkby will love you - another AIX machine! |
comment:37
Replying to @kiwifb:
I very much doubt you could build Sage on AIX. #9999 list at least some of the issues. Since I happened to create the 10,000th Sage trac ticket (#10000), I guess I should fix the fact the GSL library fails to install on AIX 5.3. I create a patch, which has been accepted upstream, but never got around to actually creating a new Sage package for GSL. Perhaps I can convince someone to review it if I do - there's an extra test in the configure script, and changes to a file with aix in the name, so is only used on AIX. Dave |
comment:38
In any case thanks for the positive review Dave. I expect aix to hurt, I don't know how much of the gentoo tree is aix safe to do a sage-on-gentoo install compared to a vanilla one but that will be interesting. While I am thinking about it, I may completely disappear for a week or 2 without warning any time now. I'll be on "paternal leave". |
comment:39
How wonderful! Another Sage devel is also on that right now, so to speak. Enjoy the experience; by the time our third came along, the hospital stay actually felt like a vacation :-) |
comment:40
Replying to @kcrisman:
Number #2, my wife was counting on the hospital stay to have a break from number #1. Unfortunately because of the earthquake the stay will have to be short (2 maternity units are shot). This is wonderfully off-topic. |
comment:41
Replying to @kiwifb:
:-) |
comment:42
Wow, this is the best off-topic side-thread on a ticket I've seen yet! Congrats! And I hope things are getting back to normal after the earthquake(s). |
comment:43
Well, after 1172 seconds, long doctests on riemann.pyx pass on my PPC machine. I'm running most tests of modules where the phrase 'import numpy' occurs as well, but this should remain robust. |
comment:44
Everything passed in calculus/, functions/, matrix/, numerical/, plot/, modules/, and stats/. That's nearly all of Numpy in Sage (rings/ has some, and a few elsewhere. PPC is fine. |
This comment has been minimized.
This comment has been minimized.
Merged: sage-4.7.alpha2 |
Changed reviewer from David Kirkby, Karl-DIeter Crisman, Ethan Van Andel to David Kirkby, Karl-Dieter Crisman, Ethan Van Andel |
New version of numpy is out: 1.5.1
SPKG: http://spkg-upload.googlecode.com/files/numpy-1.5.1.spkg
Apply:
CC: @sagetrac-evanandel @kcrisman
Component: packages: standard
Author: François Bissey, Jason Grout
Reviewer: David Kirkby, Karl-Dieter Crisman, Ethan Van Andel
Merged: sage-4.7.alpha2
Issue created by migration from https://trac.sagemath.org/ticket/10792
The text was updated successfully, but these errors were encountered: