Issue 1047 encore docs#1062
Conversation
…or edits in documentation.
…or edits in documentation.
kain88-de
left a comment
kain88-de
left a comment
There was a problem hiding this comment.
Good work only some very minor comments.
| --------- | ||
| Parameters | ||
| ---------- | ||
|
|
There was a problem hiding this comment.
This line doesn't line up with the previous.
| Returns: | ||
| --------- | ||
| Returns | ||
| ------- |
There was a problem hiding this comment.
Doesn't line up with the previous line.
| for the same method can be explored by adding different instances of | ||
| the same dimensionality reduction class. Provided methods are the | ||
| Stochastic Proximity Embedding (default) and the Principel Component | ||
| Stochastic Proximity Embedding (default) and the Principal Component |
There was a problem hiding this comment.
A side question how does your PCA compare with the new PCA analysis?
There was a problem hiding this comment.
This is a good question. The fact that there are now two versions of PCA in MDAnalysis is a bit of an accident. After early discussions on how encore could best be integrated in MDAnalysis, there was some consensus on making the dimensionality reduction and clustering code more generally available within MDAnalysis. I therefore rewrote the code so that both clustering and dimensionality got their own front-end functions, and that particular clustering and dimensionality reduction algorithms could just be plugged in - for instance those from scikit-learn. PCA was just a natural example to include. Only after doing this work did I realized that someone had coded a PCA in the mean time.
They should probably be merged at some point, but @orbeckst suggested that we stick with the two versions for this release, and then take a step back and think about some refactoring.
So, long story short. I haven't compared the two implementations. But the one in Encore is just a direct interface to the one in scikit-learn.
There was a problem hiding this comment.
So then you need to load everything into memory while the other implementation only needs to hold the current frame and can therefore estimate a PCA for even a TB trajectory.
There was a problem hiding this comment.
Right. That is at least one obvious difference.
* Fixes in Encore and MemoryReader documentation to make it render with sphinx. * Edits in Encore and MemoryReader documentation. * Renamed PrincipleComponentAnalysis to PrincipalComponentAnalysis. Minor edits in documentation. * Changed spacing issues
2 participants
Changes made in this Pull Request:
Changes to the documentation of Encore, both factual corrections and edits to improve sphinx rendering.
PR Checklist