Skip to content
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

[Suggestion] Small exports for distributed programs #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[Suggestion] Small exports for distributed programs #3

wants to merge 2 commits into from

Conversation

SGrondin
Copy link

Hi,

This is a fork used in large distributed programs where I work. It adds a Distributable class that inherits from Digest. The purpose of that class is to minimize the size of the exported state (toArray) so that a node wanting to read a percentile value can fetch lots of small internal states from each node and recompute the percentile quickly.

It implements toList(), which is a more compact version of toArray(). It uses arrays to save space on the countless mean: ..., n: .... The centroids can be pushed back into a new Distributable instance using .push(centroid[0], centroid[1]).

I have no idea if this would be useful to you or anyone else, but I'm opening this PR in case you find it interesting and/or want to merge it.

@welch
Copy link
Owner

welch commented Feb 29, 2016

Thanks! I'll give it a look later today.

@SGrondin
Copy link
Author

SGrondin commented Mar 1, 2016

To be honest, I think it's a very narrow use case, the settings are hardcoded and there's no tests. I would be surprised if you merged is as is. I opened the PR because if I do work on top of open source software I like to show the author how it's being used in case it gives them ideas :)

@welch
Copy link
Owner

welch commented Mar 2, 2016

As you say, it's not mergeable code (I'd hit you up for unit tests at least).
But it's a pretty classy way to submit a feature request 😄

I'll take this on so I can get you back to running the main line.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants