search

HdrHistogram / hdrhistogram-go

A pure Go implementation of Gil Tene's HDR Histogram.
MIT License
435 stars 64 forks source link

Make it harder for Mean() and StdDev() to overflow #21

Open cassava opened 8 years ago

cassava commented 8 years ago

This is my proposal for solving issue #20

codahale commented 8 years ago

This looks great, but the admittedly very touchy tests don’t pass. Feel free to round the stddev and I’ll gladly merge this. Thanks for the patch!

filipecosta90 commented 3 years ago

bumping this @cassava . The tests are still flaky. How do you want to proceed with it? :

:~/go/src/github.com/HdrHistogram/hdrhistogram-go$ make test
GO111MODULE=on go get -t -v ./...
GO111MODULE=on go fmt ./...
GO111MODULE=on go test -count=1 ./...
--- FAIL: TestStdDev (0.01s)
    hdr_test.go:80: StdDev was 288675.1403682712, but expected 288675.1403682715
FAIL
FAIL    github.com/HdrHistogram/hdrhistogram-go 0.153s
FAIL
make: *** [Makefile:35: test] Error 1
filipecosta90 commented 3 years ago

@ahothan we can approve this assuming we merge #40 first.

filipecosta90 commented 3 years ago

@ahothan I've confirmed that all tests pass with the current master. If we want we can merge it. wdyt?

ahothan commented 3 years ago

@filipecosta90 I was wondering if this could introduces the same kind of issue at the other end of the spectrum of possible values, when total count is extremely high and values are extremely small, but it looks like float64 can handle that in all realistic cases as the smallest number for float64 is ~10**(-308) So this patch looks good.