Telecommunication-Telemedia-Assessment / SITI

Command-line tool for calculating Spatial Information / Temporal Information according to ITU-T P.910
Other
38 stars 12 forks source link

Add mean SI/TI calculation and simplify min/max #14

Closed hofbi closed 4 years ago

hofbi commented 4 years ago

I added the average SI/TI calculation which is also performed by the siti python package (see https://github.com/slhck/siti/blob/master/siti/__main__.py#L162). The average calculation requires to store all values in a vector. As this vector is now available, I could refactor the min/max calculation which was performed on every calculation to just a single call at the end.

slhck commented 4 years ago

Looks good to me, @plebreton if fine for you we can merge!

plebreton commented 4 years ago

I performed the merge.

Thank you for the pull request! I just replaced the vector by a list to avoid re-allocations. In principle, there is also no need to keep all intermediate values if we are only after min/max and averages. But let's keep it that way for the sake of clarity.

hofbi commented 4 years ago

@plebreton Actually vector is better in terms of performance compared to the list, so I would recommend to switch back to vector. The re-allocations do not contribute that much here compared to other benefits of vector compared to list.

I created a small benchmark to proof my statement: https://ideone.com/1Szydh

Here are already some results:

Iterations: 10000000
vector [push,accumulate,total] = [84740138,64,84740202] ns
list [push,accumulate,total] = [345217665,29024218,374241883] ns

Iterations: 1000000
vector [push,accumulate,total] = [5500705,59,5500764] ns
list [push,accumulate,total] = [26504645,2983032,29487677] ns

Iterations: 100000
vector [push,accumulate,total] = [600430,84,600514] ns
list [push,accumulate,total] = [2826317,213889,3040206] ns

Iterations: 10000
vector [push,accumulate,total] = [67925,50,67975] ns
list [push,accumulate,total] = [237173,33533,270706] ns
plebreton commented 4 years ago

OK, I did not know. Thanks for the info.

I changed back the code to using a vector.