math: p_erf: Implement error function

[Adamszk]

I am adding math function to math library. Tested on stand alone location of parallella. it works fine. Please ignore my spelling error on comments in commits. The actual codes spelling are correct. Please notify me of any errors or improvement opportunities in Git set up. I am a new Git user. Thank you.

[lfochamon]

Hi! You need to provide an actual implementation of the function, not just a wrapper using math.h. The idea is that PAL will without any reference to math.h functions.

[Adamszk]

Okay.

[mateunho]

In my opinion, name can be changed to p_erf_f32.
In math.h it's named erff since it operates on float. In pal types are specified as a suffix (eg. _u64, _f32), so the additional f can be omitted.

[Adamszk]

Thanks for creative comments. I will look into that.

[mansourmoufid]

Hi @Adamszk, I think that's a good idea. If you would like to use a polynomial approximation, you can find one here: http://eliteraspberries.github.io/pal/formulas/#erf

[Adamszk]

to eliteraspberries and mateunho, thanks for comments. I looked at the formula and tested it. It does not give me a sixth decimal precision when comparing to standard erf(x) function. I think it is required for float numbers. On top of that, it has to depend on exp(x) function and I do not know how it will effect the erf(x) when exp() is modified. I am trying to write a standalone erf(x). I have been looking for fast erf algorithm with 6th decimal precision and I found one however, it still relies on exp(x). Using standard exp(x) I tested that my_erf(x) and it works works fine, but when I tried to implement my_exp(x), the precision is reduced to two decimals for one algorithm and 4 decimals to other algorithm. I just need more time to find exp(x) and implement my_erf(x) for parallella purpose and test it before I submit it. erf(x) is very sensitive to exp(x) function.

[mansourmoufid]

@Adamszk, 6 decimal digits is the best accuracy for the float type, but you don't need to be that accurate. Your function will pass the tests with an absolute error of 0.001 and a relative error of 0.00001. (Look at the compare function in the file tests/simple.c, and the values of EPSILON_MAX and EPSILON_RELMAX in tests/simple.h.)

[Adamszk]

@eliteraspberries , finally I did find the exp(x) function that works for me that gives my erf(x) precision to 6 decimals. All is left is to do a lot of pruning (I was adding and testing a lot of codes in one file) and then adapt it to parallella. Then test it with pal etc. I hope it will work fine. it is about 5 times faster than math.h erf function.

[Adamszk]

@mateunho , yes, erf(x) is the unary function. I just copy the data and modified the output. I don't know the purpose of the second column. I assume it is a second input. If it is, then when updating, I will set the second column to zero's. What is the allowed maximum rows in data file? Is the number of rows the same for all functions?

[mateunho]

@Adamszk Test data file structure consists of following columns (all of them are required):

ai	bi	res	gold
1st input	2nd input	function result	gold reference

so in case of erf() passing only ai and gold is enough, the rest can be zeroed.
Currently most test data files are about 100 lines.

[mateunho]

@Adamszk Debugging would be easier if you pushed recent changes to the github :)

@mateunho https://github.com/mateunho The code is done. Works fine
on PC but in parallella I am getting fail test message on final test.
I do not know what is the source of failure is. I have done all the
steps correctly therefore I am running out of ideas. Below is the
message I received,
CC check_p_erf_f32-simple.o
simple.c: In function 'tc_against_gold_e':
simple.c:120:5: warning: implicit declaration of function 'p_erf_f32'
[-Wimplicit-function-declaration]
FUNCTION(ai, res, gold_size);
^
CC check_p_erf_f32-p_erf.o
CCLD check_p_erf_f32
check_p_erf_f32-simple.o: In function |tc_against_gold_e':
/home/parallella/TestingPAL/pal/tests/math/simple.c:120: undefined
reference to|p_erf_f32'
collect2: error: ld returned 1 exit status
make[5]: *** [check_p_erf_f32] Error 1
make[5]: Leaving directory |/home/parallella/TestingPAL/pal/tests/math'
make[4]: *** [check-am] Error 2
make[4]: Leaving directory|/home/parallella/TestingPAL/pal/tests/math'
make[3]: *** [check] Error 2
make[3]: Leaving directory |/home/parallella/TestingPAL/pal/tests/math'
make[2]: *** [check-recursive] Error 1
make[2]: Leaving directory|/home/parallella/TestingPAL/pal/tests'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/parallella/TestingPAL/pal/tests'
make: *** [check-recursive] Error 1
parallella@parallella:~/TestingPAL/pal$

the make file in tests folder looks fine. Any suggestions?

[Adamszk]

I dont believe it is complete. I have to double check it with larger data file (more data lines). My first code was working fine with the old data file but when used more data lines to about 100, few numbers did not match. I gave up that code with a new one from scratch. It is much simpler but slower for larger numbers of a. I tested that code with old data lines of 32. Later I will retest it with more data lines of up to 100. Please see the issue with file.dat update. Any comments and constructive criticisms are welcome.

[mansourmoufid]

👍 Very accurate output, and at least 10 times faster than GNU libm on x86.

[mansourmoufid]

I have only one suggestion: to generate new test data, for input in [-3, 3].

For example, with Python:

import math
n = 100
a, b = -3.0, 3.0
x = [a + (b - a) / n * i for i in range(n)]
data = [(a, 0.0, 0.0, math.erf(a)) for a in x]
for (a, b, c, d) in data:
    print("{:.6f},{:.6f},{:.6f},{:.6f}".format(a, b, c, d))

[Adamszk]

@eliteraspberries
Thanks for feedback. I never used python but I will try to implement that in python as a learning experience.
I see additional improvement to the code. I noticed that c = 1 or -1 when a > 5 or a < -5. The current code keeps iterating even for a> 5 or a<-5 with a penalty of wasting time. I am going to implement condition for a to cut down iteration time.

[Adamszk]

@olajep
I am done with error function. Any suggestions are appreciated. Thanks to all for support.

[mateunho]

@Adamszk, I would suggest

• rebase/squash commits (currently there are 23 of them!), something like git rebase --interactive HEAD~23
• sign-off your commit, not function (see requirement), with git commit --amend --signoff

[Adamszk]

@mateunho Thanks. rebase/squash commits will take some time. Like I said before, I am new to Git.

[Adamszk]

@mateunho I am done with squashing and correcting the sign off. I hope that's everything and correctly. Again, thanks to all for support. Adieu.

[Adamszk]

@eliteraspberries
@mateunho
@lchamon
erf is still listed on pull request. Is it because it is considered incomplete?
I want to add another function. Should I make a new fork with new repository or just continue with commits with existing repository? How to test/compare a function that is not available in c++?

[mateunho]

@Adamszk

One fork is perfectly fine. The thing you need is another branch.

1. create new branch called devel
   • git checkout -b devel
2. check your remotes, if you don't have upstream pointing to PAL, then run:
   • git remote add upstream https://github.com/parallella/pal.git; git fetch upstream
3. reset the branch to the point where parallella/pal is
   • git reset --hard upstream/master
4. push devel branch to your github repo (PAL's fork), so everyone can see it
   • git push --set-upstream origin devel

After that you'll get a brand new branch synced with current state of PAL.

[Adamszk]

@mateunho

1. done
2. stuck
   parallella@parallella:/Git/pal3$ git reset --hard upstream/master
   fatal: ambiguous argument 'upstream/master': unknown revision or path not in the working tree.
   Use '--' to separate paths from revisions, like this:
   'git [...] -- [...]'
   parallella@parallella:/Git/pal3$ git reset --hard pal3/pal
   fatal: ambiguous argument 'pal3/pal': unknown revision or path not in the working tree.
   Use '--' to separate paths from revisions, like this:
   'git [...] -- [...]'
   parallella@parallella:/Git/pal3$ cd ..
   parallella@parallella:/Git$ ls
   WorkingOnPAL pal pal3
   parallella@parallella:~/Git$

what do I put in upstream/master ?

[mateunho]

@Adamszk
upstream, typically, is a name of remote repository (in case of PAL sth. like https://github.com/parallella/pal.git) and origin is your fork's url, both returned by git remote --verbose. If you don't have it then I suggest adding it (see reference).

master is the main branch of that repo

hence upstream/master

[Adamszk]

@mateunho
Done and thank you for guidance and being patient with my shortcomings. So the objective was to create a new branch devel where I do my changes. Not my Master?.
So which branch I do my next function addition? Why my erf function is not accepted? Did I miss something or done something not right?

[mateunho]

@Adamszk As a software engineer I'm used to using many branches for development purposes, master being the one for syncing with stable release. If you're interested in deeper insight into working with branches take a look at this and that.

If you want your PR to get merged quicker, ask @aolofsson or @olajep for review.

Regards,
~ Mateusz

[Adamszk]

@olajep
@aolofsson
Please review and, if acceptable, merge a pull request for math: p_erf: implement error function.

[Adamszk]

@mateunho , So which branch I am supposed to do a new code adding. Pal3/devel or Pal3/Master The devel branch does not have erf(x) I created. Should I restart the erf(x) commit?

[mateunho]

@Adamszk, if you want your code additions to be considered as a separate features, then commit them on different branches. Unless your new code is dependent on erf(), you should put it on devel or any other branch, but not master.

In general, your pull requests should be concise (not helluva lot of commits), targeted at one feature (which itself may be huge) and well described/documented. (further reading)