Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stats min is always 0 #31

Open
jfischoff opened this issue Nov 12, 2018 · 5 comments
Open

Stats min is always 0 #31

jfischoff opened this issue Nov 12, 2018 · 5 comments

Comments

@jfischoff
Copy link

jfischoff commented Nov 12, 2018

A new Distribution zeros out cMin. https://github.com/tibbe/ekg-core/blob/master/System/Metrics/Distribution.hsc#L99

AFAICT it will aways be zero if the input is positive: https://github.com/tibbe/ekg-core/blob/master/cbits/distrib.c#L24

I think cMin would be more useful it was initialized to the largest Double. Then it would show the smallest value seen if a single data point was added.

@thoughtpolice
Copy link
Contributor

thoughtpolice commented Dec 2, 2019

See also 813aa42 (& #35) which basically gets to the core of it: zero is not the identity for the Min/Max monoids, maxBound/minBound are! As expected, this same bug occurs in a "dual" way: any negative maximum value tracked by the Distribution will always be overwritten by zero.

@thoughtpolice
Copy link
Contributor

thoughtpolice commented Dec 2, 2019

Also, to be clear, my fixes are somewhat independent of the issue here. I think a better fix to the underlying issue in this ticket is to actually track whether or not we've added a first value at all. If we have not seen any values in a distribution at all, min/max should be zero. Otherwise, if at least one value has been seen, they should use ordinary semigroup logic that exists today. This results in an extra conditional but is better behavior, IMO.

The proposed solution of using maxBound :: Double as the default min value results in an edge case: if a Distribution is created and no values are added to it, but you read it, then min will come back as maxBound :: Double. I find this highly unintuitive, and is a real case that occurs: for example, if I create a Distribution and immediately attach a statsd server to it with ekg-statsd, it can (and in my case, absolutely will) send out samples to the upstream server before any values are added to the underlying Distribution. In fact, in my case, it may be many seconds before any such modification occurs, so I could get maxBound (resp. minBound) for quite a few samples.

@23Skidoo
Copy link
Collaborator

#35 has been merged (and released), leaving open so that we don't forget to apply the fix outlined in @thoughtpolice's comment.

@jprupp
Copy link

jprupp commented Feb 6, 2021

I still have this problem: min and max are always being reported as 0.0 for a distribution.

@Naveenaidu
Copy link

The problem still exists, looks like the min and max values are always zero. This can easily be tested out with the following snippet:

{-# language OverloadedStrings #-}
module EkgTest where


import System.Metrics
import System.Metrics.Distribution as Distribution

main :: IO ()
main = do
  s <- newStore
  d <- createDistribution "foo" s
  Distribution.add d 3
  Distribution.read d >>= print

The output is:

Stats {mean = 3.0, variance = 0.0, count = 1, sum = 3.0, min = 0.0, max = 0.0}

As you can notice the min and max values are zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants