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
New module complex_mpc using lib mpc for complex multiprecision arithmetic #4446
Comments
Attachment: complex_mpc.patch.gz |
Attachment: trac4446-complex_mpc.patch.gz replace the previous patch (rebase against 3.2) |
comment:2
There seems to have been some bitrot due to recent changes in Sage. I have uploaded a very slightly modified version of Philippe's patch, which applies cleanly against 3.2. A number of doctests fail for various reasons. I will try to look into this soon. |
Attachment: complex_mpc.p0.patch.gz coercion problems resolved, more functions |
comment:3
The coercion problems have been solved, all doctests now succeed in new version. The whole set of mpc functions involving complex numbers is now in the interface. |
apply after complex_mpc.p0.patch |
comment:4
Attachment: trac4446_fix.patch.gz Philippe, Great work! I'll do my best to review this for 3.2.2. Right now I notice that the patch doesn't work in 3.2.1 because of #4580. I'm attaching a tiny patch that fixes that (should be applied after complex_mpc.p0.patch. |
comment:5
There are a few small issues that I ran into, but since this is going to be (for now) an optional part of Sage and nothing depends on it, I think we can fix these later (I'm keeping a list of them so they don't get lost). However, there is a policy of 100% doctest for all new code. The coverage is now:
So the missing docstrings and doctests will have to be added before this patch can be merged. I would happily do this but the earliest I can get to it is Thursday or so (and that's being optimistic). To end on a positive note: this looks good. It will take a bit more work, but if we can show that (a) the performance is improved, or doesn't suffer too much, and (b) the switch can be made seamlessly, then it will be easy to convince people that we should switch the core of our complex numbers functionality over to MPC. |
apply after complex_mpc.p0.patch and trac4446_fix.patch |
comment:7
Attachment: trac4446_doctests.patch.gz I added a patch trac4446_doctests.patch, which does a number of things:
Note that only the last three patches should be applied, in order: complex_mpc.p0.patch, trac4446_fix.patch, and trac4446_doctests.patch |
Attachment: trac4446_norm.patch.gz |
comment:8
One more patch trac4446_norm.patch with:
|
comment:9
REFEREE REPORT:
So the only way I can see this going in like this is if it is standard. Am I missing something (probably)?
Thus until the above issues are addressed, I see no point in mpc going into sage. |
Attachment: complex_mpc.pxd.gz |
Attachment: complex_mpc.pyx.gz |
Attachment: mpc.patch.gz Attachment: mpc.pxi.gz |
comment:10
Philippe Theveny, who had a 2-year contract with us, just left, thus he won't be able to continue working on this. In case somebody wants to pursue his work, |
Attachment: mpc-0.8.1.patch.gz |
comment:48
Replying to @zimmermann6:
But you still need to create an spkg/install and spkg/standard/deps for this to be a standard package. I would attach them to the ticket.
Dave |
comment:49
Replying to @sagetrac-drkirkby:
Paul,
Dave |
apply only this patch |
comment:50
Attachment: trac4446-optional_mpc-sage4.5.2based.patch.gz This ticket is already too long, but here is another rebased version:
|
comment:51
Dave,
this should be the very first thing to do, before we invest more time in this ticket. William, are you out there? Paul |
comment:52
I'm "out there". Is it necessary and not slower than what is in Sage already? I'm willing to take your (=Paul's) word for it, assuming you say you've done some benchmarks. Also, rounding might be (vastly) better in mpc, which would be another point in its favor. |
comment:53
Replying to @williamstein:
William, here are some results of benchmarks I've done on my Core 2 Duo laptop, with Sage 4.5.3,
For the accuracy, I've computed the maximal relative error on the real or imaginary part after
I used the following programs:
The conclusion is that MPC is in the majority of the cases faster than ComplexField (the only For MPC we observe that the maximal relative error is --- as guaranteed by MPC --- always less than 1/2 ulp (unit in last place) which corresponds to a relative error of 2**(-p), i.e., 1.12e-16 for p=53 and 7.89e-31 for p=100. I thus give a positive review to Yann's patch. Paul |
Changed reviewer from William Stein, David Kirkby to William Stein, David Kirkby, Paul Zimmermann |
comment:54
Replying to @sagetrac-ylchapuy:
After applying the patch but not installing the optional package, I get
Should all of the tests in |
comment:55
Or one of
etc. |
comment:56
Yes sorry, my mistake. I won't be able to add this before next week, so if someone motivated beats me... (Paul?) |
Attachment: trac4446-mark_doctests_optional.patch.gz |
comment:57
I guess nobody has beaten me... I hope I choose the good version, it's the one used one the Mpc library web page. Back to the stuff I have to do, I'm late now...
|
comment:58
with the last patch (to be applied after the previous one), I get before installing the optional
and after installing it:
Paul |
Changed reviewer from William Stein, David Kirkby, Paul Zimmermann to William Stein, David Kirkby, Paul Zimmermann, Mitesh Patel |
comment:59
Thanks! With the package and patch installed, I get no errors with
|
comment:60
Harald, Mike, or Minh, could one of you please add http://yann.laiglechapuy.net/spkg/mpc-0.8.3-dev-svn793.spkg to the optional spkg repository? |
Merged: sage-4.6.alpha2 |
comment:62
Replying to @qed777:
Done. See the optional spkg repository at |
comment:63
Thanks, Minh. |
Herewith and there (http://www.loria.fr/~thevenyp/complex_mpc.patch 38K) is a patch with new classes using the MPC library for complex multi-precision arithmetic (see ticket #4308 for the associated spackage).
This is an adaptation of the module real_mpfr and of ComplexField and ComplexNumber classes. It adds a class MPComplexField with precision (common to both part) and rounding modes (specific to each part) and a class MPComplexNumber.
This first attempt implements only the complex arithmetic.
The test suite does fail due to coercion problems I can't solve.
CC: @robertwb @williamstein @mwhansen @sagetrac-mvngu @haraldschilly @nexttime
Component: basic arithmetic
Author: Philippe Theveny, Alex Ghitza, Yann Laigle-Chapuy
Reviewer: William Stein, David Kirkby, Paul Zimmermann, Mitesh Patel
Merged: sage-4.6.alpha2
Issue created by migration from https://trac.sagemath.org/ticket/4446
The text was updated successfully, but these errors were encountered: