Add RFGPBenchmark (#5010)

[jonpsy]

Here's the benchmark result in release build on max cpu power (using sudo cpupower frequency-set --governor performance)

After the PR:
https://pastebin.com/xefubPQ6

Before the PR
https://pastebin.com/PqneD4Nb

Benchmark for the PR: #5010
@karlnapf @gf712

EDIT: For future readers, here's the final draft for performance comparison .

Before PR:
https://pastebin.com/2ETBQgZL

After PR:
https://pastebin.com/rBwWPbqX

[gf712]

Cool! How does it compare to before your PR? :)

[jonpsy]

@gf712 , Updated it :)

[vigsterkr]

@jonpsy great work!

[karlnapf]

Alright! This is quite a speed up! I suspect the speed ups will be even larger for much larger problems. Maybe add one bigger one that takes a second or so?

You only benchmark the transform here, not the fit. Might be worth doing that too (as it will be faster as well with the code you wrote, interesting to know by how much)

[jonpsy]

Right, we're planning to benchmark

1. transform
2. apply_to_feature_vector
3. fit

We've cleared part 1. Two more to go ✌

[karlnapf]

I think 1 and 3 are the important ones.

BTW does transform call apply_to_matrix once or apply_to_vector in a loop?

[jonpsy]

The former

[karlnapf]

ok good. I think there is not really a need to benchmark the vector version. But fit is definitely interesting

[jonpsy]

Updated benchmark results:

After PR
https://pastebin.com/rBwWPbqX

Before PR
https://pastebin.com/XJ1vjwd9

This is the maximum load my already fragile laptop can achieve before causing a house fire. I'm still not sure about the Fit misbehaving in before PR.

[karlnapf]

This is the maximum load my already fragile laptop can achieve before causing a house fire. I'm still not sure about the Fit misbehaving in before PR.

🤣

[karlnapf]

No need to go bigger :)
Really great improvements. You see it was worth taking the time to do this properly.

Not sure about the fit thingi...but we cannot compare like this obviously ...

[karlnapf]

Is it maybe that fit doesnt do anything before the PR because stuff was done lazily? This looks like timings for a no-op...
If that is the case, then the numbers above might be wrong in the sense that are not comparing apples and apples because before the PR, transform included computing a fit

[gf712]

Is it maybe that fit doesnt do anything before the PR because stuff was done lazily? This looks like timings for a no-op...
If that is the case, then the numbers above might be wrong in the sense that are not comparing apples and apples because before the PR, transform included computing a fit

In that case might be best to just compare fit + transform?

[karlnapf]

yes that might be easiest.... maybe best to start with that. Though it is also interesting how this decomposes ...

[jonpsy]

I can assure you both the transform methods start at an equal footing. The prior transform method do not use fit inside it. We are winning, there's no doubting that.

About the misbehaving Fit method, I think I've cracked the code. Thing is, test_rfinited() do not let you calculate fit again for same set of parameters, it just returns the old ones. Due to this, we get say ~2ms on first itr then 0ms for the next 200+ iter. The average of which is 0 ms. Hence the erratic behaviour, voila!

[gf712]

I can assure you both the transform methods start at an equal footing. The prior transform method do not use fit inside it. We are winning, there's no doubting that.

About the misbehaving Fit method, I think I've cracked the code. Thing is, test_rfinited() do not let you calculate fit again for same set of parameters, it just returns the old ones. Due to this, we get say ~2ms on first itr then 0ms for the next 200+ iter. The average of which is 0 ms. Hence the erratic behaviour, voila!

In that case wouldn't it be better to move the object instantiation to the loop? It will add a bit of noise in the benchmark, but imo the result would be more comparable

[jonpsy]

Or.... We could tweak the prior method a little ;) (by removing test_rfinited() )

Here's the result of the prior PR fit method : https://pastebin.com/7hNSAPwq

Another clear win. Victory feels nice, ain't it?

[gf712]

why not auto feats = std::make_shared<DenseFeatures<float64_t>>(mat);, like below?

[jonpsy]

We're fitting and transforming two different matrix.

[karlnapf]

That is feat the relative gains in fit are even stronger that in transform.
Let's merge it then?

@jonpsy make sure to take a moment to remember what the reasons of those performance gains were:

• SIMD vectorized operations rather than loops over components
• Matrix-vector products rather than loops over vectors
• And much simpler code

[gf712]

@jonpsy great work! :)