Added `derive_surface` and `get_positions` with tests.

[ojeda-e]

Function core_fast_leaflets split into three functions:

• [x] get positions for each atom in the atom group for each frame.
• [x] identify the grid cell for each coordinate.
• [x] calculate the average z for the atom group.

Tests added:

• [x] test_get_positions using dummy coordinates for beads 0 to 8, all of them with z=10:

  o ______ o _____ o _______ |
  |   (6)  |  (7)   |   (8)  |
  o ______ o _____ o _______ |
  |   (3)  |  (4)   |   (5)  |
  o _______o ______ o ______ |
  |   (0)  |  (1)   |   (2)  |
  o ______ o ______ o ______ |

  Using the same number of beads in grids,

• [x] test_avg_unit_cell added for two systems.

  1. z values of z=10
  2. z values as below:

     o ______ o _____ o _______ |
     | (z=10) | (z=20) | (z=30) |
     o ______ o _____ o _______ |
     | (z=10) | (z=20) | (z=30) |
     o _______o ______ o ______ |
     | (z=10) | (z=20) | (z=30) |
     o ______ o ______ o ______ |

  3. z values of z=10 and number of beads per unit cell as shown below:

     o ____ o ____ o ___ |
     |   2  |  1   |  1  |
     o ____ o ____ o ___ |
     |   1  |  2   |  1  |
     o ____ o ____ o ___ |
     |   1  |  1   |  2  |
     o ____ o ____ o ___ |

• [x] test_derive_surface added for same dummy_coordinates as in test_get_positions.

[pep8speaks]

Hello @ojeda-e! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file membrane_curvature/lib/mods.py:

Line 40:32: W291 trailing whitespace Line 43:53: W291 trailing whitespace Line 45:53: W291 trailing whitespace Line 49:78: W291 trailing whitespace Line 53:29: W291 trailing whitespace Line 70:9: E741 ambiguous variable name 'l' Line 92:29: W291 trailing whitespace

Comment last updated at 2021-07-05 03:21:02 UTC

[codecov[bot]]

Codecov Report

Merging #40 (4567ed1) into main (97713b3) will decrease coverage by 1.33%. The diff coverage is 88.00%.

[ojeda-e]

After reviews, functions get_positions and grid_map were replaced by single lines in the function derive_surface. Two tests were added for this function. One using previously used small gro file, and passing coordinates as np.arrays in mda.Universe.

[orbeckst]

If you leave the voodoo cast here, raise an issue to investigate further. From the amount of comments you got here you can see that nobody likes code that we don’t understand.

Am 7/3/21 um 12:45 schrieb Estefania Barreto-Ojeda @.***>:

 @ojeda-e commented on this pull request.

In membrane_curvature/lib/mods.py:

• grid_count_frames = np.zeros([n_cells, n_cells])
• factor = np.float32(n_cells / max_width) Thanks for highlighting the docstrings, max_width is not int type, it's float. I'll update them. I don't see anything wrong with keeping it anyway. I'll leave and will re-evaluate in a performance test.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[ojeda-e]

Thanks @orbeckst . Issue opened (#42), the line was changed to calculate with factor = n_cells / max_width only.

[ojeda-e]

In this PR:

• [x] Function derive_surface slightly modified from suggestion by @lilyminium to consider different max_width in x and y dimensions. i.e. derive_surface(n_cells, selection, max_width_x, max_width_y).
• [x] Smoke test added for rectangular and square grid using pytest.mark.parametrize for get_z_surface.
• [x] Previous tests modified accordingly to changes from get_z_surface.

Different binning for x and y will be addressed in upcoming issue #35