MDAnalysis / membrane-curvature

MDAnalysis tool to calculate membrane curvature.
https://membrane-curvature.readthedocs.io/
GNU General Public License v3.0
29 stars 6 forks source link

Add test for the generation of z_ref (core_fast_leaflet) on toy system #22

Closed lilyminium closed 3 years ago

lilyminium commented 3 years ago

The code to transform xy coordinates to grid cells is important and modular enough that you could keep it as a function in the AnalysisBase refactor. Therefore, it should get tested extensively to make sure that it is correct, especially as the regression tests for the mean and gaussian curvature in #13 depend on z_ref coordinates that were presumably generated this way. Writing the test could be done before, or in parallel, with starting to outline the AnalysisBase re-factor.

Instead of testing the generation of z_ref on the GRO file you have uploaded, a simpler test that is more easy to humanly verify would be a small toy system of fake coordinates (maybe 5-12 coordinates per axis for a suitable corresponding number of cells). You could use pytest.mark.parametrize to test multiple inputs (or you could have separate tests for each one if that's more your style!). You might need to have at least two tests though, one for tests that currently pass and one for tests that are expected to fail (marked with XFAIL, e.g. those with PBC) These inputs could include:

For example, I would check that a system of [(-1, 0), (0, 0), (1, 0), (2, 0), (3, 0)] gives a grid with the (-1, 0) coordinate last and only one y-row. This smaller system is much easier to understand than a real membrane, and defines expectations such as PBC.

ojeda-e commented 3 years ago

I think this issue is fixed with the tests added in PR #48
Would you please confirm this is ok, @lilyminium?

lilyminium commented 3 years ago

If you add a test for a rectangular grid then I think it's all covered!

ojeda-e commented 3 years ago

Thanks. I added one rectangular test as part of a pytest parametrize to check MembranCurvature.results.average_z_surface, block in lines 307-324. If that’s not enough I can add a couple for mc.results.mean and mc.results.gaussian. :)