Have GetValueAtQuantile return min and max for quantiles 0 and 100 #83
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
A brief description of the change being made with this pull request.
Have
GetValueAtQuantilereturn min and max for quantiles 0 and 100.DDSketchWithExactSummaryStatisticsalready store the min and max values and clamp the values within that range.But we can still have quantile 100 return values less than the max.
This change ensures that quantile 0 returns min, and quantile 100 returns max.
Motivation
What inspired you to submit this pull request?
I am working on using DDSketch to report some stats, and found for integer inputs
DDSketchwould return non-integer values. For example the data set { 28 } (one value) returns p100 of 27.5. I pursued changing the relativeAccuracy parameter, then realized fixing p100 to returnGetMaxseemed beneficial for all users.Additional Notes
Anything else we should know when reviewing?
FWIW, one might want to investigate by keeping the new test lines in
ddsketch_test.goand temporarily reverting the changes inddsketch.go. I found testing easier by only runningTestNormal, and limitingtestSizes={3}andtestQuantiles = []float64{0, 1}, andtestCases = []testCase...to only the first one withexactSummaryStatistics: true.