search

coarse-graining / cgnet

learning coarse-grained force fields
BSD 3-Clause "New" or "Revised" License
57 stars 26 forks source link

Fixed bug in neighbor masking code #147

Closed brookehus closed 4 years ago

brookehus commented 4 years ago

Development:

The neighbor masking procedure did two things: (1) it set the index of every masked neighbor to zero, and (2) it created a boolean mask of which neighbors to consider. (1) is actually never used in the code to enforce the masking; everything is calculated and then the masked stuff is hidden.

However, the neighbors were already zero-indexed. So this setting to zero in part (1) meant that a bunch of neighbors were becoming different beads.

I went through and tested the equivalence of this current change (not setting masked neighbors to zero anymore) with the old code and everything seems to be fine. You can check out the branch schnet-bug-fix for that code if you want to do any of your own testing, which adds an option set_to_zero for various functions.

Let me know what you think.

nec4 commented 4 years ago

Awesome - will take a look!

nec4 commented 4 years ago

All tests pass - LGTM! Great catch.