Addition to benchmark suite about tinyexpr HOT 17 CLOSED

Thanks for including TinyExpr in your benchmark. You've raised a lot of issues - I probably won't be able to address them all in one go.

Your usage: I haven't had time to look over your benchmark code. I'll try to provide feedback later.

Your code changes: I took a look at the three lines you've indicated changes too, and it's not immediately obvious to me why any were made. Could you please provide explanations? Were you having trouble with the code as-is? What compiler are you using?

The incorrect results: In the repeated exponentiation examples, TinyExpr uses a left-to-right evaluation. This is documented in the Readme (under "Functions supported"). It's simply a trade-off where I'm valuing size and simplicity. In the log() examples, I think maybe you're expecting the natural log where TinyExpr uses base-10 log? TinyExpr provides ln() for the natural log function. (Again, this is documented in the Readme, but you could change it very easily in tinyexpr.c if you like).

As for the other incorrect results, it's hard to know what is going wrong since you don't include variable values, calculated results, or expected results. Please provided complete examples with the full inputs, returned values, and expected results - then I can get to the bottom of it.

Couple notes: TinyExpr was not designed to be fast, it was designed to be small and simple. I think it would be nice if you could add into your library listing the programming language and an approximate lines-of-code count for each tested library. I would greatly appreciate this, as I consider those to be TinyExpr's main features.

Anyway, I appreciate you bringing these issues to my attention, and it's interesting to see how TinyExpr stacks up against the competition. I'll try to help how I can with getting the issues worked out.

from tinyexpr.

Your usage: I haven't had time to look over your benchmark code. I'll try to provide feedback later.

If you do get a chance do have a look as it'll answer some the questions you have noted.

Your code changes: I took a look at the three lines you've indicated changes too, and it's not immediately obvious to me why any were made.

The first one is related to compilation of the expression when there's trailing white-space, it returns an error. As an example, when passed the string "1 " it returns an error, adding the switch-case for null-terminator to set the tok_end in the state seems to work - though I'm not sure if it's how you would have done it, as I'm not too familiar with the code-base, just got into it enough to make it work for the benchmark suite.

The other two are related to annoying compiler warning diagnostics that indicate potential UB - the compiler used was msvc 2015 SP3.

Warning  C4090 'initializing': different 'const' qualifiers ..\tinyexpr\tinyexpr.c  394
Warning  C4090 'initializing': different 'const' qualifiers ..\tinyexpr\tinyexpr.c  364
Warning  C4090 'initializing': different 'const' qualifiers ..\tinyexpr\tinyexpr.c  379
Warning  C4200 nonstandard extension used: zero-sized array in struct/union ...\tinyexpr\tinyexpr.h  38

This could be an issue of differences between C and C++ rules or even a broken MSVC compiler. Best thing to do would be build with both GCC and Clang with pedantic et al enabled, to really know what is going on.

TinyExpr provides ln() for the natural log function. (Again, this is documented in the Readme, but you could change it very easily in tinyexpr.c if you like).

Instead of modifying code for what is essentially a static change, would it be possible to have a more configurable interface, perhaps an option struct that denotes one or the other (and possibly other options too), then during compilation it can be decided based on the nature of the option selected which one will be used log/ln or log10. As an example in ExprTk, users can configure how sin (or any other reserved function) is implemented in order to use degrees as input instead of the default of radians:
https://github.com/ArashPartow/exprtk/blob/master/readme.txt#L2326

As for the other incorrect results, it's hard to know what is going wrong since you don't include variable values, calculated results, or expected results.

It's all explained here:
https://github.com/ArashPartow/math-parser-benchmark-project#the-rounds

But simply put, in the column that comes after the time (ns) column, TinyExpr returned 0, whereas all of the other parsers returned -0.81084173.... - so it should be pretty obvious what's going on.

That being said, you're saying it's a difference in how TinyExpr orders associativity for the exponentiation operation - which I guess is fine, though all the parsers in the benchmark suite seem to implement the standard PEDMAS (or BEDMAS) convention.

from tinyexpr.

perhaps an option struct that denotes one or the other (and possibly other options too), then during compilation it can be decided based on the nature of the option selected which one will be used log/ln or log10.

I'm fairly certain you can already achieve this by overriding the built in log and ln using custom functions, seen that the parser checks first for the variables and only if that fails goes on to search among the builtin ones though I did not test this.

from tinyexpr.

you can already achieve this by overriding the built in log

You are correct. The builtin functions can easily be overridden, and log can be overridden like this:

   te_variable vars[] = {
        {"a", &a},
        {"b", &b},
        {"log", log, TE_FUNCTION1 | TE_PURE}
    };

from tinyexpr.

But simply put, in the column that comes after the time (ns) column, TinyExpr returned 0, whereas all of the other parsers returned -0.81084173.... - so it should be pretty obvious what's going on.

Well, I assure you that TinyExpr is not returning 0 for -a^-b where a=1.1 and b=2.2. TinyExpr calls to the C pow() function internally. I think that TinyExpr is returning the same result as the C code for pow(-1.1, -2.2), which on my machine is NaN.

from tinyexpr.

all the parsers in the benchmark suite seem to implement the standard PEDMAS (or BEDMAS) convention.

I don't think it's as standard as you suggest. Matlab, Octave, and Excel, for example, evaluate multiple exponentiation from left-to-right, the same as TinyExpr. I do understand the argument for right-to-left evaluation, I just don't think it's worth the complexity trade-off right now.

from tinyexpr.

TinyExpr is returning the same result as the C code for pow(-1.1, -2.2), which on my machine is NaN.

When you have a moment if you could debug it a bit in your own environment and let us know if you get the same results, because if there is something wrong with what I'm doing, I'd like to fix it up.

I don't think it's as standard as you suggest. Matlab, Octave, and Excel, for....

From octave-online.net:

octave:1> a=1.1, b=2.2, -a^-b
a =  1.1000
b =  2.2000
ans = -0.81084

From Wolfram-Alpha:
https://www.wolframalpha.com/input/?i=a+%3D+1.1,+b+%3D+2.2,+-a%5E-b

Python:

a=1.1
b=2.2
print -a**-b

sh-4.3$ python main.py
-0.810841732005

Ruby:
http://codepad.org/m2AIE5de

a=1.1
b=2.2
print -a**-b
------------------------
-0.810841732005177

That said, If there are any other mathematical or general computing environments you'd like me to try out, do let me know.

from tinyexpr.

When I mentioned Octave, I was talking specifically about left-to-right associativity for exponentiation. It's relevant for tests such as a^2.2^3.3. If you run that in TinyExpr and Octave, I think you'll get the same answer. Python, Ruby, Wolfram-Alpha, etc will give a different answer.

The -a^-b issue has nothing to do with parsing or order of operations. TinyExpr just calls the C pow() function. Please see: https://ideone.com/yLmOvW

from tinyexpr.

The -a^-b issue has nothing to do with parsing or order of operations. TinyExpr just calls the C pow() function. Please see: https://ideone.com/yLmOvW

As I had suggested to you earlier: "debug it", as when I debugged it, I noticed the first parameter passed to pow was a garbage DEN value. So hopefully without being too repatative:

When you have a moment if you could debug it a bit in your own environment and let us know if you get the same results, because if there is something wrong with what I'm doing, I'd like to fix it up.

I was talking specifically about left-to-right associativity for exponentiation.

The issue, again seen when debugging it, seems to be that the unary negation on "a" is being evaluated before the exponentiation. In bracketed form it should be: -(a^-b) or -(1.1^-2.2), where as TinyExpr seems to be doing: (-a)^-b

Look, this is your library, and at the end of the day you can dictate how it behaves. So that being said, I'll leave it up to you and loop back on TinyExpr towards the end of the year, if any changes have been applied which resolve the issues noted fine, otherwise I'll remove it from the benchmark.

from tinyexpr.

Just as a heads up, I'm not trying to antagonize you, and I hope you don't read this comment as it being too defensive or aggressive or something of the sort.

With that out of the way, I really don't understand how the parsing order of the library is relevant to a benchmarking tool as long as it isn't plain wrong. If you are trying to inform users of the quickest parser shouldn't these decisions be irrelevant? Wouldn't users of said benchmarking suite want to know how quick the library is, even if it makes different documented choices?

By taking this stance of "if you don't make these decisions about how to parse ambiguous expressions, I'm removing you from the benchmark" you are just being a bit intolerant in my view.
I understand that you need to validate the results, though couldn't your tool be a bit smarter by recognizing differing approaches to solving ambiguous expressions; or by just adding braces so expressions aren't ambiguous; or even saying some of the tests don't apply to this specific library because it made different choices, but still include it in the other tests?

All that being said it's your benchmarking tool and it should be as tolerant or as flexible as you want it to, though I think that doesn't need being said seen you know its yours and that only you have merging privileges :).

As to changing TinyExpr to accommodate this "problem", it's up to codeplea, were it my project, I would add compile-time USE flags for the different methods, though that would add a complexity to the code that is outside of the philosophy of this library.

Independently of any of changes being made, in the linked repo, it states new parsers are always welcome, but does not specify anywhere the parsing priorities required by you for the parser to be accepted. If you chose to remove TinyExpr from the benchmark, I would advise you to also specify the inflexibility of the suite on the documentation because as it stands, it looks as if it accepts all Mathematical Expression Parsers and not just the ones that make specific decisions. Your standards should, in my opinion, be stated.

from tinyexpr.

The issue, again seen when debugging it, seems to be that the unary negation on "a" is being evaluated before the exponentiation. In bracketed form it should be: -(a^-b) or -(1.1^-2.2), where as TinyExpr seems to be doing: (-a)^-b

I see. That didn't occur to me. I'll think on it.

For what it's worth, TinyExpr's behavior matches spreadsheets like Excel and

Google Sheets: https://docs.google.com/spreadsheets/d/1W9_HATGwKpshhU7rg5mcySwJq9hfK_jJbbO3KUphqEM/edit?usp=sharing

Ethercalc: https://ethercalc.org/b1ovpyz66i05

It also matches Tcl: https://ideone.com/eDWF4d

It also matches bash scripts: https://ideone.com/J8mDWm

I'll think about making the behavior optional, but I think it's pretty unfair to call it wrong. It is all documented in the Readme. TinyExpr was originally made to power a spreadsheet-like application (which is also why log is log10).

from tinyexpr.

it's up to codeplea, were it my project, I would add compile-time USE flags for the different methods,

I'm a bit torn on this. Changing the -a^-b issue is actually pretty simple now that I know what he's looking for. It's the 2^3^4 issue that I think would add the most complexity. In any case, I'd want to make it compile-time optional as you suggest. Excel compatibility is important to me.

from tinyexpr.

I've decided to go ahead and implement the changes for compatibility with Python, Ruby, Mathematica, etc.

from tinyexpr.

I pushed an update. If you uncomment the following lines near the top of tinyexpr.c, I think it'll behave closer to how you expect:

/* #define TE_POW_FROM_RIGHT */
/* #define TE_NAT_LOG */

Can you try it and let me know?

from tinyexpr.

I updated the benchmark code and added a pull request.

It seems to pass everything except for the logic operators, which TinyExpr doesn't implement. Neither do many of the other libraries you're using too.

from tinyexpr.

Thanks for the PR, I've merged it and it works well - though I needed to make a couple of changes to TinyExpr in order to get rid of the last remaining warning diagnostics.

With regards to the inequalities that's fine, most of the parsers don't implement them. The original author of the benchmark Ingo Berg thought it would nice to have, in order to see what the different parsers were capable of doing etc.

As for not all the libraries, that is true, though most of the ones that have problems specific to parsing and evaluation haven't been touched for a long time and the authors don't seem to be contactable anymore - though for the most part (excluding weird), the overwhelming majority of expressions are correctly parsed and evaluated by the overwhelming majority of parsers.

That being, said including TinyExpr into the benchmark, gives a new set interesting results, specifically to the differences of implementation efficiency when comparing C to C++.

Typically one would trot out the qsort vs std::sort example where the comparator in the C++ version is inlined making for an efficiency gain over the C version. Now we can demonstrate the power of function hoisting during optimisation and the pitfalls of pointer indirection due to closures.

In the following, two expressions are evaluated a+1 and abs(a+1), in the first instance, TinyExpr is quite competitive, though in the second expression something changes...

Expression 1 of 2: "a+1"; Progress: ############
[01] muparserSSE          (    4.393 ns,       2.099999904632568359, 5299999.952316284179687500)
[02] ExprTkFloat          (    4.631 ns,       2.099999904632568359, 5300000.071525573730468750)
[03] ExprTk               (    5.296 ns,       2.100000000000000089, 5300000.000073863193392754)
[04] atmsp 1.0.4          (   19.068 ns,       2.100000000000000089, 5300000.000073863193392754)
[05] muparser 2.2.4       (   19.255 ns,       2.100000000000000089, 5300000.000073863193392754)
[06] MTParser             (   19.937 ns,       2.100000000000000089, 5300000.000073863193392754)
[07] TinyExpr             (   19.977 ns,       2.100000000000000089, 5300000.000073863193392754)
[08] muparser 2.2.4 (omp) (   22.842 ns,       2.100000000000000089, 5300000.000073863193392754)
[09] MathExpr             (   23.296 ns,       2.100000000000000089, 5300000.000073863193392754)
[10] FParser 4.5          (   24.301 ns,       2.100000000000000089, 5300000.000073863193392754)
[11] Lepton               (  182.719 ns,       2.100000000000000089, 5300000.000073863193392754)
[12] muparserx            (  217.585 ns,       2.100000000000000089, 5300000.000073863193392754)

Expression 2 of 2: "abs(a+1)"; Progress: ############
[01] muparserSSE          (    4.536 ns,       2.099999904632568359, 5299999.952316284179687500)
[02] ExprTk               (    8.933 ns,       2.100000000000000089, 5300000.000073863193392754)
[03] ExprTkFloat          (    9.492 ns,       2.099999904632568359, 5299999.952316284179687500)
[04] atmsp 1.0.4          (   22.233 ns,       2.100000000000000089, 5300000.000073863193392754)
[05] MTParser             (   26.298 ns,       2.100000000000000089, 5300000.000073863193392754)
[06] muparser 2.2.4 (omp) (   26.313 ns,       2.100000000000000089, 5300000.000073863193392754)
[07] muparser 2.2.4       (   27.170 ns,       2.100000000000000089, 5300000.000073863193392754)
[08] MathExpr             (   27.535 ns,       2.100000000000000089, 5300000.000073863193392754)
[09] FParser 4.5          (   28.405 ns,       2.100000000000000089, 5300000.000073863193392754)
[10] TinyExpr             (   42.007 ns,       2.100000000000000089, 5300000.000073863193392754)
[11] Lepton               (  201.420 ns,       2.100000000000000089, 5300000.000073863193392754)
[12] muparserx            (  235.864 ns,       2.100000000000000089, 5300000.000073863193392754)

Where all the parsers did increase evaluation times in the second expression, TinyExpr was the only one to have such a massive difference.

The reason as far as I can tell, is that when all the other parsers either called std::abs or fabs, the compiler optimiser was able to detect the abs call and instead of making a call into libm, hoisted the function and inlined an efficient implementation equivalent to abs.

Where as in TinyExpr because the call to abs is hidden behind a closure, the optimiser is unable to get around the inherent pointer indirection - same issue seen with qsort and its comparator.

In theory you could fix the situation for abs, but then you'd loose the ability for your users to override abs with their implementation, you'd also have the same problems with the other functions (log, sin, cos et al) you provide.

In closing isn't it nice to now have a means to immediately be able to test and improve your changes in TinyExpr against a wide range of parser implementations with varying levels of complexity? :D

from tinyexpr.

In closing isn't it nice to now have a means to immediately be able to test and improve your changes in TinyExpr against a wide range of parser implementations with varying levels of complexity? :D

Yes, it is nice. I really want to keep TinyExpr simple (that's the whole point of the project), but if I have any free time I'm seriously considering forking TinyExpr into a new project where I go for speed. I have several ideas on how to speed things up.

from tinyexpr.