-
Notifications
You must be signed in to change notification settings - Fork 26
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
[ddsketch] Add re-mapping function #34
Conversation
ddsketch/store/store.go
Outdated
@@ -26,6 +26,7 @@ type Store interface { | |||
AddBin(bin Bin) | |||
AddWithCount(index int, count float64) | |||
Bins() <-chan Bin | |||
ForEach(f func(b Bin)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it ForEach(f func(b Bin) bool)
to allow early stopping? E.g., we iterate while true
is returned.
// ChangeMapping changes the store to a new mapping. It is not the conversion that minimizes the loss in relative | ||
// accuracy, but it avoids artefacts like empty bins that make the histograms look bad. | ||
// scaleFactor allows to scale out / in all values. (changing units for eg) | ||
func (s *DDSketch) ChangeMapping(newMapping mapping.IndexMapping, positiveStore store.Store, negativeStore store.Store, scaleFactor float64) *DDSketch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it explicit that negativeStore
and positiveStore
are meant to be initially empty and distinct store instances? Can we also mention that s
is unchanged by the operation and that a new distinct sketch is returned?
Adds function to be used when converting from one mapping to another one.
It uses bucket overlapping to decide the buckets to assign the values to.
It's not the conversion that minimizes the lost relative accuracy, but it avoids artefacts like empty bins and it produces histograms that look nice.