math: add inverse error function

[btracey]

I'd like to request the addition of the inverse error function (erfinv) and the
complement, erfcinv to the math package to go along with erf and erfc. The inverse error
function comes up most commonly in statistics (quantile of the normal distribution being
the prime example), though it makes frequent appearances elsewhere. It is not a function
in cmath, but it is a function in the cuda math library (not to mention matlab, scipy
and mathematica).

[rsc]

Comment 1:

It's not clear whether we want to expand the set of functions. We've resisted before,
keeping it to just what the standard C library has.
We should decide one way or the other for Go 1.3.

Labels changed: added priority-later, go1.3maybe, removed priority-triage.

Status changed to Thinking.

[btracey]

Comment 2:

Have you had a lot of requests? In the issue tracker I only see one request (for Round). 
I don't think it has to be a binary (some additional functions can be added without
adding all possible functions). Erfinv and Erfcinv are a good fit for the math package
because they
 a) compliment functions aleady in the math package (right now you can use the math package to compute the cumulative distribution of a Gaussian at x but not inverse problem of finding the x which gives that cdf) and 
and
b) are not a one or two line function to implement (erf and erfc combine for about 200
lines of code)

[rsc]

Comment 3:

Labels changed: added release-none, removed go1.3maybe.

[rsc]

Comment 4:

Labels changed: added repo-main.

[markthelaw]

Hi I need inverse error function for my monte carlo project. It would be great if I don't have to code it myself.

[markthelaw]

I wrote a inverse error function here: https://github.com/markthelaw/GoStatHelper/blob/master/StatUtil/StatUtil.go
Based off Apache commons-math

[rsc]

It seems like this is something we could add, with signatures

func Erfinv(x float64) float64
func Erfcinv(x float64) float64

We can't take Apache-licensed code but if someone would like to write one from scratch or using one of the sources we already derive from in package math, that would be fine.

-rsc for @golang/proposal-review

[btracey]

I didn't see these new comments on the issue (didn't realize I wasn't subscribed).

Just this week I submitted an optimized version of the NormalQuantile function to gonum/mathext (https://github.com/gonum/mathext/blob/master/erf.go). I adopted it from code.google.com/p/probab, written by @ThePaw and @skelterjohn , who may be willing to transfer the copyright. It's a simple transformation to go from NormalQuantile to Erfinv.

[btracey]

Otherwise it's all already BSD code if that's okay.

[skelterjohn]

I am happy to transfer any copyrights. I don't recall writing that particular function, so it might be up to @ThePaw.

…

On Tue, Feb 21, 2017 at 1:45 PM, Brendan Tracey ***@***.***> wrote: Otherwise it's all already BSD code if that's okay. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6359 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUZ1qsScBmHWbjNnJ1cFVqggpa7z5A9ks5rezCygaJpZM4ErTFf> .

[btracey]

Otherwise, the algorithm used in that PR is given in https://www.jstor.org/stable/pdf/2347330.pdf if someone would like to adopt it de novo (I probably can't because of already adapting the probab code). However, I did compare this algorithm with several other published ones and it seems to be the best.

[lakshayg]

I would like to start contributing to go and this seems like a relatively simpler task. Should I give this is a try?

[ALTree]

@lakshayg it's labelled HelpWanted and it seems like nobody is working on this, so I'd say it's all yours.

seems like a relatively simpler task

Localized, I'd say. I'm not sure about simple, this kind of function is often tricky to implement correctly.

[lakshayg]

I have written a C++ implementation of erfinv based on the the algorithm given in https://www.jstor.org/stable/pdf/2347330.pdf. I am attaching the plots of error abs(x - erfinv(erf(x))). If this looks reasonable then I will go ahead and implement the function in go. I have implemented the function in go. Will send if for review on gerrit soon.

The error for floats is of the order 10^-7 and for doubles it is of the order 10^-16. Feedback will be appreciated.

[rsc]

@lakshayg, thanks for working on this. Until there is assembly, don't write:

func Erfinv(x float64) float64

func erfinv(x float64) float64 {

Instead, write only:

func Erfinv(x float64) float64 {

When you are using the declaration pair above, you have to provide assembly that at least glues them together (see stubs_*.s in that directory).

[lakshayg]

@rsc Yes, I just figured that out. I have added the erfinv_stub.s. All the tests are passing.
I am struggling with git-codereview at the moment as I have not been able to set it up properly. Is there another way to send the code for review?

[ALTree]

@lakshayg you don't need any stub because you're implementing this in pure go. Just write

func Erfinv(x float64) float64 {

and remove the stubs.

Is there another way to send the code for review?

Unfortunately no, there isn't.

[lakshayg]

I ran go get -u golang.org/x/review/git-codereview to install git-codereview as described here: https://golang.org/doc/contribute.html#git-codereview_install. After running this command, when I run git codereview help, I get an error message git: 'codereview' is not a git command. See 'git --help'.. Any ideas on how to fix this or anything I might be missing (This is the first time I'm using go)?

[cespare]

@lakshayg perhaps you don't have your $GOPATH/bin in your $PATH? (See https://golang.org/doc/install#install.)

[gopherbot]

CL https://golang.org/cl/46990 mentions this issue.

[lakshayg]

@cespare That fixed the problem, thanks! Just submitted the code for review.

[rsc]

Thanks. We're in the Go 1.9 release freeze right now and won't review it until the Go 1.10 cycle opens (~Aug 1), but we appreciate the contribution.

[ianlancetaylor]

Can anybody on this issue review https://golang.org/cl/46990? Thanks.

[ALTree]

Erfinv(x float64) float64 done in the CL above, Erfcinv(x float64) float64 still to do.

[lakshayg]

@ALTree erfcinv(x) = erfinv(1-x). Would it suffice to add just this?

[lakshayg]

Just sent in a CL for erfcinv: https://go-review.googlesource.com/57090

[gopherbot]

Change https://golang.org/cl/57090 mentions this issue: math: implement the erfcinv function