Faster numericq - Githubissues

mmatera commented 3 years ago

This PR keeps from #1569 those changes related to N and NumericQ. Now,

NumericQ uses the definitions instead of just a subset of predefined numeric functions.
A bug in the Definitions.__init__ method was corrected, which allows distinguishing rules of the form N[expr_]:=(definition) of other definitions.
Expression("N",expr_,prec_).evaluate(evaluation) now is a function that avoid to pass through the pattern matching based calling way.

rocky commented 3 years ago

@mmatera I am seeing negligible changes for this N[] alone. Is this expected?

report

Here is the raw data from that:

Mathics git repo Mathics3/mathics-benchmark/Mathics at d929af
500 iterations of N...
  1.689227 secs for: N[Pi, 50]
  0.395894 secs for: N[1/7]
  1.868149 secs for: N[1/7, 5]
  0.160950 secs for: N[a] = 10.9
  2.420790 secs for: N[a + b]
  1.561825 secs for: N[a, 20]
  0.167243 secs for: N[a, 20] = 11
  0.505543 secs for: N[f[a, b]]
  0.249549 secs for: N[c, p_?(#>10&)] := p
  1.772729 secs for: N[c, 3]
  3.599174 secs for: N[c, 11]
  0.237608 secs for: N[d] ^= 5;
  0.121222 secs for: NValues[d]
  0.239138 secs for: N[e] = 6;
  0.216735 secs for: N[e]
  0.261976 secs for: N[e[f]] = 7;
  0.518711 secs for: N[g[x_, y_], p_] := x + y * Pi /; x + y > 3
  0.702546 secs for: N[g[1, 1]]
  2.083610 secs for: N[g[2, 2]] // InputForm
  3.364433 secs for: N[Exp[0.1], 50]
  6.668159 secs for: N[Exp[1.0`20], 50]
  1.545872 secs for: N[1.012345678901234567890123, 20]
  1.568412 secs for: N[I, 30]

Mathics git repo Mathics3/mathics-benchmark/Mathics at 1aa72b
500 iterations of N...

  1.650870 secs for: N[Pi, 50]
  0.391735 secs for: N[1/7]
  1.855669 secs for: N[1/7, 5]
  0.160432 secs for: N[a] = 10.9
  2.439698 secs for: N[a + b]
  1.567786 secs for: N[a, 20]
  0.170654 secs for: N[a, 20] = 11
  0.517632 secs for: N[f[a, b]]
  0.253449 secs for: N[c, p_?(#>10&)] := p
  1.782839 secs for: N[c, 3]
  3.633280 secs for: N[c, 11]
  0.237080 secs for: N[d] ^= 5;
  0.121803 secs for: NValues[d]
  0.236840 secs for: N[e] = 6;
  0.224925 secs for: N[e]
  0.261064 secs for: N[e[f]] = 7;
  0.518955 secs for: N[g[x_, y_], p_] := x + y * Pi /; x + y > 3
  0.705104 secs for: N[g[1, 1]]
  2.081307 secs for: N[g[2, 2]] // InputForm
  3.400784 secs for: N[Exp[0.1], 50]
  6.688979 secs for: N[Exp[1.0`20], 50]
  1.571455 secs for: N[1.012345678901234567890123, 20]
  1.598649 secs for: N[I, 30]

rocky commented 3 years ago

Here are benchmarks I am seeing for NumericQ. Is this what you get too?

report

Raw data:

Mathics git repo Mathics3/mathics-benchmark/Mathics at d929af
500 iterations of NumericQ...
  0.039970 secs for: NumericQ[2]
  0.039239 secs for: NumericQ["x"]
  0.042377 secs for: NumericQ[x]
  0.044534 secs for: NumericQ[Pi]
  0.060444 secs for: NumericQ[{1, 2}]
  0.210654 secs for: NumericQ[3+I]
  0.112572 secs for: NumericQ[5!]
  0.593598 secs for: NumericQ[Sqrt[Pi]]

Mathics git repo /src/external-vcs/github/Mathics3/mathics-benchmark/Mathics at 1aa72b
500 iterations of NumericQ...
  0.039018 secs for: NumericQ[2]
  0.039248 secs for: NumericQ["x"]
  0.042687 secs for: NumericQ[x]
  0.045282 secs for: NumericQ[Pi]
  0.060504 secs for: NumericQ[{1, 2}]
  0.210136 secs for: NumericQ[3+I]
  0.113324 secs for: NumericQ[5!]
  0.604614 secs for: NumericQ[Sqrt[Pi]]

rocky commented 3 years ago

@mmatera @TiagoCavalcante I am seeing the same kind of negligible changes when use the benchmark code on https://github.com/mathics/Mathics/pull/1571 . So right now I think there is something wrong with the benchmark routine.

A guess is not running against the right branch.

mmatera commented 3 years ago

At this point, I didn't expect a big improvement. In a further PR I will put the changes that could improve the performance.

TiagoCavalcante commented 3 years ago

@mmatera @TiagoCavalcante I am seeing the same kind of negligible changes when use the benchmark code on #1571 . So right now I think there is something wrong with the benchmark routine.

A guess is not running against the right branch.

I don't know, what I have seen is that in my machine the order of the branches matter, e.g.: if you benchmark first master then more-small-tweaks the result is one, and when you do in the inverse order, the result is another. Also about it: maybe I'll discard some changes because the rules are a bit more complex with them and I couldn't see positive or negative differences (as each time I run the benchmark the result was one) for some changes. Anyway, I'll benchmark today.

I'll revise bench.py today, but maybe this is a Python problem, I mean, its overhead covers the improvements up. Maybe we can benchmark better with Pyston/PyPy.

TiagoCavalcante commented 3 years ago

@rocky and @mmatera can the apply_N be used in every place that has Expression(SymbolN, ...).evaluate(evaluation) or Expression("N", ...).evaluate(evaluation)?

rocky commented 3 years ago

@rocky and @mmatera can the apply_N be used in every place that has Expression(SymbolN, ...).evaluate(evaluation) or Expression("N", ...).evaluate(evaluation)?

Yes, but please don't make exhaustive changes until we understand what the performance benefit of this is, and have benchmarking to the point where we can run it reliably. Thanks.

TiagoCavalcante commented 3 years ago

Yes, but please don't make exhaustive changes until we understand what the performance benefit of this is, and have benchmarking to the point where we can run it reliably. Thanks.

Ok, I'll wait until the things get stable.

rocky commented 3 years ago

@mmatera @TiagoCavalcante After working on the benchmark programs to make sure we reinstall code before running tests, I am still not seeing much improvement:

https://github.com/Mathics3/mathics-benchmark/pull/15 has the revised code.

report

The thing I am curious about is this the same kind of thing you all are seeing?

rocky commented 3 years ago

@mmatera @TiagoCavalcante In trying one of the benchmarking ContinuedFraction here is what I get:

report

python ./mathics_benchmark/bench.py -v ContinuedFraction faster_numericq
Mathics git repo /src/external-vcs/github/Mathics3/mathics-benchmark/Mathics at d929af
500 iterations of ContinuedFraction...
  0.267295 secs for: ContinuedFraction[Pi, 10]               
  1.627767 secs for: ContinuedFraction[(1 + 2 Sqrt[3])/5]    
  0.850787 secs for: ContinuedFraction[Sqrt[70]]             

Dumping information to file ./mathics_benchmark/../results/ContinuedFraction.json
Mathics git repo /src/external-vcs/github/Mathics3/mathics-benchmark/Mathics at 1aa72b
500 iterations of ContinuedFraction...
  0.229929 secs for: ContinuedFraction[Pi, 10]               
  1.592049 secs for: ContinuedFraction[(1 + 2 Sqrt[3])/5]    
  0.835065 secs for: ContinuedFraction[Sqrt[70]]

I am wondering if this is consistent with what you all see too.

~~The bar charts seem off because the first change should have something like a 14% improvement but it isn't noted and it doesn't look like that either.~~ Sorry, I should have been comparing the last item. This looks correct. I am surprised I didn't see the percentages.

TiagoCavalcante commented 3 years ago

I am surprised I didn't see the percentages.

That is a bug in my code, it is looking for the absolute difference to show the percentages, but it should be looking for percentage difference (when it shows the percentage they are correct, but the threshold for showing them is wrong).

I'll fix it now.

Sorry for the silly mistake.

rocky commented 3 years ago

I am surprised I didn't see the percentages.

That is a bug in my code, it is looking for the absolute difference to show the percentages, but it should be looking for percentage difference (when it shows the percentage they are correct, but the threshold for showing them is wrong).

I'll fix it now.

Sorry for the silly mistake.

No problem - and thanks for fixing. One less mystery...

TiagoCavalcante commented 3 years ago

I have run it 3 times.

The last time: