Skip to content

removed obsolete functions from lib.distances#2073

Merged
zemanj merged 9 commits intoMDAnalysis:developfrom
zemanj:issue-2072-lib-distances
Sep 13, 2018
Merged

removed obsolete functions from lib.distances#2073
zemanj merged 9 commits intoMDAnalysis:developfrom
zemanj:issue-2072-lib-distances

Conversation

@zemanj
Copy link
Copy Markdown
Member

@zemanj zemanj commented Sep 13, 2018

Fixes #2072

Changes made in this Pull Request:

  • removed the now obsolete functions calc_distance, calc_angle, and calc_dihedral from lib.distances
  • replaced all of their occurrences in the code by suitable calls to calc_bonds, calc_angles, and calc_dihedrals, respectively
  • transformed the corresponding tests into tests of calc_bonds, calc_angles, and calc_dihedrals for single coordinate input
  • updated the docstrings of calc_bonds, calc_angles, and calc_dihedrals to better reflect the possibility of single coordinate input (also related to issue MDAnalysis.lib.distances needs rework #2046)

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@zemanj
Copy link
Copy Markdown
Member Author

zemanj commented Sep 13, 2018

Regarding my latest commit: The precision used to be 3 decimals in the tests for calc_angle and calc_dihedral. When I applied these tests to calc_angles and calc_dihedrals, I increased the precision to 5 decimals, but that fails in one of the tests for numpy 1.10.4. I therefore decreased the precision again to 4 decimals.
That's in fact pretty inaccurate, and only happens if distances are computed with PBC.
I suspect that is due to the way the minimum image convention is implemented in calc_distances.h, where the inverse box is supplied in single precision.
Changing this to double precision wouldn't really affect performance since it is promoted to double in the calculation anyway.

@zemanj
Copy link
Copy Markdown
Member Author

zemanj commented Sep 13, 2018

Ok this seems good to go.

Copy link
Copy Markdown
Member

@richardjgowers richardjgowers left a comment

Choose a reason for hiding this comment

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

Cool ty @zemanj

@zemanj
Copy link
Copy Markdown
Member Author

zemanj commented Sep 13, 2018

@richardjgowers The tests are still quite an unstructured mess though...

@zemanj zemanj merged commit 259f778 into MDAnalysis:develop Sep 13, 2018
@zemanj
Copy link
Copy Markdown
Member Author

zemanj commented Sep 13, 2018

@richardjgowers Thanks for reviewing!

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.

2 participants