Skip to content

Updated PSA module #438

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

Merged
merged 19 commits into from
Sep 21, 2015
Merged

Updated PSA module #438

merged 19 commits into from
Sep 21, 2015

Conversation

sseyler
Copy link
Contributor

@sseyler sseyler commented Sep 16, 2015

Changes to the PSA module now reflect proposed features in the PSA paper (in review). These additions include:

  • Three new distance functions: two based on averages (hausdorff_avg and hausdorff_wavg), one based on computing nearest neighbor pairs and distances (hausdorff_neighbors)
  • Two global functions in the psa module for computing msd (get_msd_matrix, used by all distance functions) and the dimensions of numpy arrays of trajectories (get_coord_axes)
  • A new plotting method plot_annotated_heatmap to generate annotated heat maps (in addition to the heat map-dendrograms)
  • A new analysis method in the PSA class class to perform nearest neighbors/Hausdorff pairs analysis
  • A new plotting method in the PSA class class to generate plots of nearest neighbors distances as a function of (normalized) frame progress
  • A class PSAPair to facilitate the storage and extraction of heterogeneously sized data generated by nearest neighbors/Hausdorff pairs analysis
  • Additional getter methods in PSA class
  • Various changes/tweaks to documentation

@richardjgowers richardjgowers self-assigned this Sep 17, 2015
@richardjgowers richardjgowers added this to the 0.12 milestone Sep 17, 2015
@richardjgowers
Copy link
Member

Heya

Add changes to CHANGELOG please

@kain88-de
Copy link
Member

some tests would be nice as well.

@dotsdl
Copy link
Member

dotsdl commented Sep 17, 2015

@kain88-de I agree. I don't think this analysis module has any tests yet, but they should be added before this is merged. @sseyler, can you add a test class to test_analysis.py that ensure the class returns expected values for, say, an ADK transition compared to itself? It's not ideal, but I think to test this more strictly we'd need at least two different ADK transitions in the test data. Perhaps you could add a second one? No more than 10 frames or so for space considerations.

@orbeckst
Copy link
Member

@sseyler , you can add a second AdK DIMS trajectory to the test data, they are small.

@sseyler
Copy link
Contributor Author

sseyler commented Sep 18, 2015

@kain88-de @dotsdl Most definitely. Minimal tests will be added very soon so that it can be merged; as planned, a full test suite will be added in the near future.

list of indices representing the row-wise order of the objects
after clustering
from matplotlib.pyplot import figure, savefig, tight_layout, clf
import seaborn as sns
Copy link
Member

Choose a reason for hiding this comment

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

You might need to put seaborn in as an optional dependency here and maybe document that it's potentially needed.

Copy link
Member

Choose a reason for hiding this comment

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

Does seaborn change the matplotlib style also when imported in a module? If yes that might lead to surprises for some users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richardjgowers Got it.

@kain88-de seaborn does change the default matplotlib style, unless one uses import seaborn.apionly. This was what I did in the plot_annotated_heatmap method, but not the plot_nearest_neighbors method. I am going to change this so that plot_nearest_neighbors behaves the same way as plot_annotated_heatmap and doesn't catch users by surprise.

Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend using seaborn.apionly in general, since this means a user's chosen style isn't disrupted by using this method.

@sseyler
Copy link
Contributor Author

sseyler commented Sep 18, 2015

@kain88-de @richardjgowers I'm happy to convert to numpy-style docs in the near future. Just trying to get a working version of this module out so those who come across it in the associated paper can start tinkering.

@richardjgowers
Copy link
Member

@sseyler just tell me when you're happy with it and I'll merge and release 0.11.1 to pypi

@sseyler
Copy link
Contributor Author

sseyler commented Sep 21, 2015

@richardjgowers I will be rolling out periodic enhancements (and fixes) in the near future, but I'm happy with it as it currently stands (once I get the final Travis CI build to not fail).

@sseyler
Copy link
Contributor Author

sseyler commented Sep 21, 2015

@richardjgowers The tests appear to be failing, but neither I nor @dotsdl can figure out exactly why. It appears it might have something to do with coverage checks.

@richardjgowers
Copy link
Member

Coverage got bumped to version 4.0 in the last few days. Probably gotta rename something in coveragerc (or install the old version in travis)

e: fixed it, so stuff should/might/could/won't (delete according to optimism level) work

richardjgowers added a commit that referenced this pull request Sep 21, 2015
@richardjgowers richardjgowers merged commit 58484f5 into MDAnalysis:develop Sep 21, 2015
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.

6 participants