Conversation
absmall
left a comment
There was a problem hiding this comment.
Hi, sorry for not noticing this, I don't monitor github much anymore.
This change will have no effect - the job of allocate_markers on line 84 is to increase the size of the dn array by 3, adding three more entries at the end. markers will point to the start of these three entries.
The markers shouldn't necessarily be at the end - they should be spread somehow throughout the existing markers depending on the order in which add_quantile is called for the various quantiles. So we can't possibly get the markers in the right order here, we don't even try, we just put the three markers in somehow.
Then on line 91, the call to update_markers sorts all the markers. So they should end up in the right order after that call
I think it should be quantile/2.0 first and then quantile in function add_quantile. In addition, in the result function you get dn[(marker_count - 1) / 2], so it must calculate to the position of quantile but in this code is quantile/2.0. Furthermore, this PR following the P^2 algorithm as documented in "The P-Square Algorithm for Dynamic Calculation of Percentiles and Histograms without Storing Observations," Communications of the ACM, October 1985 by R. Jain and I. Chlamtac.