Allow custom stat selection #4
Conversation
It seems that it should be up to the caller to determine significance of the statistics calculated by Bear.
This patch extends bear:get_statistics/1 to allow the user to specify which statistics are desired. This defaults to the full set available. This patch also removes the scan_res2 tuple and makes the second-pass calculation and list sorting lazy - if a user doesn't request variance, skew, etc., then the second pass won't be done; similarly with percentiles and sorting. I'm not thrilled with the massive bear:compute_statistics/4 definition, but the alternatives I considered mostly involved nasty function exports and unnatural sigs. Breaking changes: 1. The median stat has been replaced by a percentile at 0.5 2. Percentiles are now specified as floats
|
Nice, I like this patch. 31a11f9: lgtm I'll need to do a bunch of testing from folsom to make sure things don't break. 3008a2c: The trick with the STATS_MIN checks are that those functions can blow up if there are too few data points. Not sure of a better way to guard against this sort of thing without thinking about it more. Let me know if you have any thoughts. 5225512: lgtm, also need to make sure this change doesn't break things in folsom before I can merge. Also, happy to see a gorgeous Koco-esque commit message. :D Thanks Ben! |
|
Cool deal. I'm planning to submit a patch for Folsom that supports these changes tomorrow (?). I'll also dig deeper in to the effects of 3008a2c, I'm sure there's a cleanish way to handle that. |
|
@joewilliams Any motion on this? |
|
I think this is waiting on @banjiewen's support in folsom before merging here. @seancribbs is this something you would like independent of folsom? |
|
No, but it's unclear to me how I couldn't use this manually, even with folsom? |
|
Much interesting patch, when reading the code of bear I immediatly wanted to propose a patch to specify which percentile is compute. I'll wait to see if this patch is accepted before propose my patch. |
|
@banjiewen any interest in perusing this PR further? |
|
#13 has been merged which should do what this PR does. |
This patchset extends bear:get_statistics/1 to allow the user to specify
which statistics are desired. This defaults to the full set available.
This patch also removes the scan_res2 tuple and makes the second-pass
calculation and list sorting lazy - if a user doesn't request variance,
skew, etc., then the second pass won't be done; similarly with
percentiles and sorting.
I'm not thrilled with the massive bear:compute_statistics/4 definition,
but the alternatives I considered mostly involved nasty function exports
and unnatural sigs.
Breaking changes:
I've included a couple of cleanup commits as well. If 3008a2c is controversial
I wouldn't be opposed to removing it; it seemed sensible and made my other
work somewhat simpler.