-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
implement numerical evaluation of erf at complex arguments via mpmath algorithm #1173
Comments
This comment has been minimized.
This comment has been minimized.
comment:1
See also |
comment:2
The following has a GPL'd implementation in c: http://velveeta.che.wisc.edu/octave/lists/archive/octave-sources.1998/msg00032.html |
comment:3
This looks like the wya to go: Josh Kantor:
|
comment:4
Numpy and Sage numbers now seem to work well, at least to some extent:
Is it time to wrap this now, two years after opening? |
comment:5
This can also be done by mpmath in arbitrary precision, see
|
comment:6
Replying to @aghitza:
And erf already has an _eval_f method, so maybe this could be changed to use mpmath? Even a loss in speed would be worth having the complex values. This could apply to error_fcn/erfc and others as well. |
comment:7
Update - yes, we should definitely do this because of the complex values - just had a support request about not being able to do
and then use n() because of this (partly). See also #9044. |
comment:8
Okay, here's v0.prealpha0.0_1a of a patch which uses mpmath to get complex and arbitrary-precision behaviour for erf. (If it works out we can do error_fcn the same way, as noted by kcrisman). Unfortunately it does introduce a speed regression:
I attempted to fix this by using the old approach for the default precision, but it usually wound up being more expensive to do the tests. Someone with better speed-fu is encouraged to look at it. I haven't finished a long testall yet, so there's probably something somewhere which breaks, but here's the first attempt. |
Author: D. S. McNeil |
comment:9
Attachment: trac_1173_add_complex_argument_erf.patch.gz The patch looks really good. The only problem I spotted after a quick reading is that the code blocks in the documentation should be after AFAIR, mpmath now supports extracting the precision from the input type in Sage. It is not necessary to do this in each calling function any more. I don't have an example handy though. For examples of fast methods to check the type of the input you can look at |
comment:10
Okay, so just to be clear: EXAMPLES and TESTS don't need trailing colons, whether double or single (although double does seem pretty common; is it okay to use it?), but I should definitely use a double colon after test descriptions, e.g.
instead of
That's easy enough to fix. The other two will take a bit more work, but I'll see what I can find. Examples to pattern after are very welcome. |
comment:11
Usually we use one colon after |
comment:12
It seems like most of the speed decrease isn't due to anything I'm doing in I'm at a loss for ways to proceed that don't involve modifying function.pyx, cythonizing, or wrapping the entire function to patch the holes. |
Reviewer: Burcin Erocal |
comment:13
Replying to @sagetrac-dsm:
The speed problems can be solved by replacing the zero test I'd be happy to give positive review to a patch with these changes... :) |
comment:14
I'd say that it should also add one very minor additional doctest, for |
comment:15
Here's another thing that should be added, reported by one of the PREP workshop participants before this patch was in:
So Pari was not handling big number in erf before. With this patch, though
Since none of the doctests in the current patch seem to test "big" real input to erf, we should add this too. |
Work Issues: add erf(sqrt(2)) and erf(45000).n(), formatting, perhaps speed |
comment:16
This should also solve #11626, I think? |
Dependencies: #11513 |
comment:18
So, to review:
This would make this depend on #11513. I'm still a little skeptical on this; will we really not get any wrong answers with that? |
comment:19
Replying to @kcrisman:
the current patch already contains examples with prec=100, both for real and complex numbers, Paul |
comment:47
no I'm not on IRC. Ok, then I'll use 4.8.alpha6 instead. Paul |
comment:48
Replying to @zimmermann6:
It certainly isn't out yet (and maybe it will never even exist if rc0 solves all our problems). The easiest way to find out the latest development release is http://www.sagemath.org/download-latest.html, accessible as www.sagemath.org -> Download -> Development Release. |
comment:49
OK, the feeling here at SD35.5 is to hijack this ticket (change the ticket description) to rebase the work done by DSM on top of Sage-4.8.alpha6 (rather than making a new ticket) so the work and documentation improvements here are preserved and compatible with #11948. Please pipe up if you're following this ticket and have an opinion. |
comment:50
Fine by me. |
This comment has been minimized.
This comment has been minimized.
comment:53
With respect to the move call patch, see also #13933. |
comment:54
Apply trac_1173_complex_erf_v3.patch |
This comment has been minimized.
This comment has been minimized.
Changed dependencies from #11513 to none |
Changed author from D. S. McNeil to none |
Changed reviewer from Burcin Erocal, Benjamin Jones to Karl-Dieter Crisman |
When implemented, this would work:
Apply patch trac_1173_complex_erf_v3.patch attached to this ticket to the Sage library.
CC: @benjaminfjones
Component: calculus
Keywords: sd35.5
Reviewer: Karl-Dieter Crisman
Issue created by migration from https://trac.sagemath.org/ticket/1173
The text was updated successfully, but these errors were encountered: