search

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

Fixed sign of mean curvature #75

Closed ojeda-e closed 3 years ago

ojeda-e commented 3 years ago

The sign in the equation of mean curvature was wrong, indeed. Thanks, @lilyminium for catching this. The overall term used to calculate mean curvature was quite messy, and it didn't match the one I added to the docstrings.

I added two main changes in this PR:

To clarify, with the change of the sign 3 tests failed: test_analysis_mean, test_mean_curvature_all, test_mean_curvature_small. As you can verify, the change in the expected value of H is the sign, exclusively. These three tests were updated accordingly.
Just in case, I double-checked the term of Gaussian curvature, and it looks OK.

Since this change has an impact on the first tutorial included in docs, I opened #74 to update the tutorial once this PR is approved. I'm very sorry for this terrible mistake, @lilyminium @orbeckst. I hope these changes fix #70.

Please let me know if I missing anything else here.

codecov[bot] commented 3 years ago

Codecov Report

Merging #75 (da3d3b8) into main (4bb851e) will not change coverage. The diff coverage is 100.00%.

ojeda-e commented 3 years ago

Hi @lilyminium, would you please take a look at the changes pushed here? :) Thanks.

lilyminium commented 3 years ago

Sorry for the delay. Looks good to me!