michellab / Sire

Sire Molecular Simulations Framework
http://siremol.org
GNU General Public License v3.0
95 stars 26 forks source link

Behavior of Logic_mix_lam #378

Closed kexul closed 2 years ago

kexul commented 2 years ago

Dear sire developers: I've seen several comments in the https://github.com/michellab/Sire/blob/devel/corelib/src/libs/SireMove/openmmfrenergyst.cpp code says:

//JM 9/10/20 multiply Logic_mix_lam by * 0 instead of max(lam,1.0-lam)

But the actual behavior is max(lam, 1.0-lam): https://github.com/michellab/Sire/blob/6ff6b04d56a3b76fb18f7ba2317557d15cd41cb6/corelib/src/libs/SireMove/openmmfrenergyst.cpp#L517

Is this a deprecated change and you just forgot to remove the comment?

jmichel80 commented 2 years ago

Thanks for raising an issue. I will post an update tonight

jmichel80 commented 2 years ago

Hi @kexul commit https://github.com/michellab/Sire/commit/db45952c6614083f9e78c3854d4f945d592b2cea adds again to devel functionality that had been lost during a previous commit. The modification to Logic_mix_lam should improve numerical stability of alchemical free energy simulations at lambda values != [0.0, 1.0] for perturbations involving simultaneously groups of atoms disappearing and appearing in the same part of a scaffold.

Tagging @halx and @annamherz @JenkeScheen as these changes may have an impact on their projects (and @lohedges for reference)

kexul commented 2 years ago

Thanks for the quick update and detailed information. 😄

lohedges commented 2 years ago

Thanks for reporting @kexul, and for the quick fix @jmichel80.