refactor: Clarify forces and derivatives #1852
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #1837.
This PR clarifies forces vs derivatives in interatomic (through-space) force calculations. While the forces calculated throughout were correct at the endpoint, the signs of various terms were mixed throughout.
Rationale
If the energy of a given interaction between two unconnected atoms at a given distance
r
isE = U(r)
(for instance via the Lennard-Jones potential) then the force acting on those atoms is equal to the negative of the derivative of the energy, i.e.F = -dU/dr
. For a pair of atomsi
andj
at a distancer
the vector between them isvij = r(j) - r(i)
, and the forces acting on them are given byF(i) = -F.vij
andF(j) = F.vij
.So, as you can see there are plenty of opportunities for absorbing (mistakenly or otherwise) negatives into equations! This PR tidies up the following things and gives us consistency ahead of moving the functions around in the next PR.
PairPotential
class functionsanalyticShortRangeForce()
andanalyticCoulombForce()
actually just returned the derivatives of the functions (i.e. the negation was missing). These functions are used to seed the tabulated potentials.dUFull_
were, as the variable name suggested, the derivatives and not the force, but thePairPotential::force()
function returns the actual values without a negation, and so was returning the derivatives and not the force.ForceKernel::forcesWith(out)Mim
functions correctly calculated the vectors the forces were acting on asi -> j
as written above, but added to atomi
and subtracted from atomj
, providing the final correcting "reversal" of the signs.Outcome
Once all of these aspects were fixed and made consistent all tests passed save for one -
MDModule.BenzeneRestart
- where the forces calculated were slightly different from the reference values in the test data. This turned out to be an age-old bug where the corrective force term in the shifted truncation scheme was correctly used for the energy (upon which tabulated simulation forces are created) but not the analytic forces. This did not affect simulation results.