Invalid .hgrm output produced #39

Closed
mattwarren opened this Issue Jan 9, 2017 · 2 comments

Projects

None yet

2 participants

@mattwarren
Member

The logic for `IsLastValue()' is incorrect and can flag multiple values as the last value.

This causes the incorrect output shown below (note multiple lines with the last column missing):

       Value     Percentile TotalCount 1/(1-Percentile)

       1.000 0.000000000000    7604459           1.00
       1.000 0.100000000000    7604459           1.11
       1.000 0.200000000000    7604459           1.25
       1.000 0.300000000000    7604459           1.43
       1.000 0.400000000000    7604459           1.67
       1.000 0.500000000000    7604459           2.00
       1.000 0.550000000000    7604459           2.22
       1.000 0.600000000000    7604459           2.50
<SEVERAL ROWS REMOVED FOR BREVITY>
     383.000 0.999998283386    9999983      582542.22
     453.000 0.999998474121    9999985      655360.00
     511.000 0.999998664856    9999987      748982.86
     537.000 0.999998855591    9999990      873813.33
     672.000 0.999999046326    9999991
     777.000 0.999999141693    9999992
   18143.000 0.999999237061    9999993
  208127.000 0.999999332428    9999994
  224639.000 0.999999427795    9999995
  229759.000 0.999999523163    9999996
  229759.000 0.999999570847    9999996
  230271.000 0.999999618530    9999997
  230271.000 0.999999666214    9999997
  258943.000 0.999999713898    9999998
  258943.000 0.999999761581    9999998
  258943.000 0.999999785423    9999998
  275711.000 0.999999809265    9999999
  275711.000 0.999999833107    9999999
  275711.000 0.999999856949    9999999
  275711.000 0.999999880791    9999999
  275711.000 0.999999892712    9999999
  282111.000 0.999999904633   10000000
  282111.000 1.000000000000   10000000
#[Mean    =        1.714, StdDeviation   =      205.505]
#[Max     =   282111.000, Total count    =     10000000]
#[Buckets =           14, SubBuckets     =         2048]

I'm pretty sure the fix is to make IsLastValue look like this (i.e using double.Epsilon):

public bool IsLastValue()
{
    return Math.Abs(PercentileLevelIteratedTo - 100.0D) < double.Epsilon
}
@LeeCampbell
Collaborator

Thanks for the bug @mattwarren. I will try to replicate it, but it looks like you are right. In the mean time are you able to provide a test to reproduce the bug?

@LeeCampbell LeeCampbell added a commit to LeeCampbell/HdrHistogram.NET-1 that referenced this issue Jan 10, 2017
@LeeCampbell LeeCampbell BUG fix for IsLastValue. Closes #39 ea5fd88
@LeeCampbell
Collaborator

PR is up with a test that I think replicates the bug. Have updated the code as suggested and it is good. I have no idea why I used the magic number 0.0001 before.

I have assigned you as the reviewer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment