choderalab / protons

OpenMM testbed for constant-pH methodologies.
http://protons.readthedocs.io/
MIT License
20 stars 13 forks source link

Array copy bugfix #126

Closed bas-rustenburg closed 6 years ago

bas-rustenburg commented 6 years ago

This uses deepcopy for numpy arrays used inside attempt_state_change. Potentially the main cause of #124

A short description of the problem:

If you index a list in python with my_list[:], this returns a copy of the entire list. For numpy arrays, it returns a reference to the array. This leads to unexpected side effects when copying is intended. I've replaced array slices with deepcopy in several places in the code where copy was the intended behavior.

jchodera commented 6 years ago

You've probably gone a little overkill here, but there shouldn't be any issues with this. Thanks for fixing this!

codecov-io commented 6 years ago

Codecov Report

Merging #126 into master will increase coverage by 0.02%. The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
+ Coverage   59.65%   59.68%   +0.02%     
==========================================
  Files          36       36              
  Lines        5109     5112       +3     
==========================================
+ Hits         3048     3051       +3     
  Misses       2061     2061
Impacted Files Coverage Δ
protons/app/driver.py 85.49% <100%> (ø) :arrow_up:
protons/app/samsreporter.py 95.5% <100%> (+0.05%) :arrow_up:
protons/app/ncmcreporter.py 77.35% <50%> (+0.21%) :arrow_up:
protons/app/titrationreporter.py 85.18% <66.66%> (+0.13%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1da1678...b1489e1. Read the comment docs.

bas-rustenburg commented 6 years ago

I'm going to rerun those ion simulations and see if this fixes the accumulation bug.