search

welch / tdigest

tdigest: javascript implementation of Dunning's T-Digest for streaming quantile approximation
MIT License
68 stars 10 forks source link

Issue-#4: Add Support for saving state #5

Open daxbert opened 8 years ago

daxbert commented 8 years ago

Here are the changes to support saving state in TDigest.

I'm not very familiar with javascript unit testing. Would appreciate any guidance you may have.

daxbert commented 8 years ago

note: I didn't bump the version as I wasn't sure what you'd prefer.

welch commented 8 years ago

Thanks for the PR!

I'm going to be swamped for a few days before I'll get a close look at it. But it seems more complicated than need be. I believe you can capture the needed state by just dumping the centroids using toArray, and restore it by calling push_centroid on each item in order (no compression will be triggered). Let me know what you think.

daxbert commented 8 years ago

Ah, I'll give that a try. If it gives the same results I'll cancel the PR.

On Jul 18, 2016 11:51 PM, "Will Welch" notifications@github.com wrote:

Thanks for the PR!

I'm going to be swamped for a few days before I'll get a close look at it. But it seems more complicated than need be. I believe you can capture the needed state by just dumping the centroids using toArray, and restore it by calling push_centroid on each item in order (no compression will be triggered). Let me know what you think.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/welch/tdigest/pull/5#issuecomment-233546859, or mute the thread https://github.com/notifications/unsubscribe-auth/AGaSPFAe3zRn6guGVrYFyJEEonU4kyLwks5qXHQAgaJpZM4JPOGF .