Skip to content

Fix `adjust_momenta` of `FixCom`

Yuji Ikeda requested to merge yuzie007/ase:adjust-momenta-fixcom into master

First MR relevant to #1285.

FixCom should make the center-of-mass velocity zero (This is also consistent with the RATTLE algorithm), but at the moment, this is not correctly implemented. This MR fixes this issue.

The first commit is for adding the test to demonstrate the issue.

The second commit fixes the issue above.

The third commit is to fix the default adjust_momenta in the (essentially abstract) class FixConstraint. At the moment, adjust_momenta is simply redirected to adjust_forces. This is however reasonable only for some type of subclasses (IndexedConstraint etc.) and in general not (FixBondLengths etc). To let new constraint developers notice about adjust_momenta I would make it as an abstract method (to force the developers to always implement this method). This commit may be optional but I feel it reasonable.

Note: Just to preserve the present behavior, I also re-implement adjust_momenta for some subclasses in the same way as the original FixConstraint (i.e. redirected to adjust_forces). I am however not immediately sure if some of them are reasonable (or correct) (e.g. for ExternalForce, should adjust_momenta really be the same as adjust_forces?) Since the motivation of this MR is just to fix FixCom, I will postpone thinking of this point...

Merge request reports