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

adding cylindrical grouping method algorithm #361

Merged
merged 25 commits into from Jul 15, 2017

Conversation

Projects
None yet
2 participants
@nickhand
Member

nickhand commented Jul 6, 2017

No description provided.

nickhand added some commits Jun 28, 2017

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Jul 6, 2017

Is the algorithm invariant with different number of ranks?

The parallel algorithm looked a bit scary. Could you add a sentence or two in the docstring / comments about the key assumption that guarantees correct result ?

@nickhand

This comment has been minimized.

Member

nickhand commented Jul 6, 2017

It's not working quite yet but I think I know the issue

rainwoodman and others added some commits Jul 7, 2017

re-worked CGM method that seems to be working
* a lot more testing and this seems to work
* takes advantage of some pandas magic for speed-ups
@nickhand

This comment has been minimized.

Member

nickhand commented Jul 13, 2017

ok @rainwoodman I'll merge this if it looks good to you

@nickhand nickhand merged commit 1fd10d7 into bccp:master Jul 15, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 95.018%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment