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

Documentation restructuring, some fixes, clustering docs #57

Merged
merged 5 commits into from Apr 23, 2020

Conversation

clonker
Copy link
Member

@clonker clonker commented Apr 23, 2020

  • The documentation is now split into apidocs and a more narrative documentation
  • the more narrative documentation is composed out of jupyter notebooks which are converted into html by the nbsphinx sphinx extension
  • added first draft of clustering narrative documentation
  • AMMs and OOMs are now part of the apidocs
  • in mini batch clustering: when calling fit() the previous behavior was falling back to ordinary k-means. Now it takes shuffled samples of the dataset and performs clustering on these mini batches

[capi] offer include directories
[cluster] mini-batch kmeans makes shuffled batching over dataset when calling fit
@clonker clonker requested a review from brookehus April 23, 2020 10:52
@brookehus
Copy link
Collaborator

awesome, will check this out today!!

Copy link
Collaborator

@brookehus brookehus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions but nothing major.

I looked at the clustering notebook here because git didn't want to render it in the PR diffs

I'm not looking at this with a copy editing mind, just basic stuff:

  • API docs link didn't work for me
  • Why does a fixed random seed make it only "quasi" reproducible?
  • Do you want the notebooks to have citations? Could cite something with MSMs
  • I love the visualization in block 11
  • I would appreciate more informative plot titles - e.g. in the plots in blocks 8, 11, 15, 17, and 24, to distinguish them
  • Can a custom metric only be defined in cpp or is python fine too?
  • I like this example a lot

@clonker
Copy link
Member Author

clonker commented Apr 23, 2020

  • quasi reproducibility: i will remove the quasi although it is in fact quasi, even ify ou fix the seed you can get different results on different machines, depending on implementation specifics of the underlying random engine
  • the notebooks have citation keys - i will add msm citations as soon as im doing the msm docs
  • will add more descriptive plot titles, thanks!
  • custom metric is cpp only

@brookehus
Copy link
Collaborator

awesome good to merge pending plot titles!

@brookehus
Copy link
Collaborator

New titles look great! I like how you did it

@clonker
Copy link
Member Author

clonker commented Apr 23, 2020

Thanks! Will merge then once CI gives green light

@clonker clonker merged commit c9f79eb into deeptime-ml:master Apr 23, 2020
@clonker clonker deleted the docs branch April 23, 2020 15:22
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