Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Clarify forces and derivatives #1852

Merged
merged 8 commits into from
May 9, 2024

Conversation

trisyoungs
Copy link
Member

@trisyoungs trisyoungs commented Apr 12, 2024

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 is E = 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 atoms i and j at a distance r the vector between them is vij = r(j) - r(i), and the forces acting on them are given by F(i) = -F.vij and F(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.

  1. The PairPotential class functions analyticShortRangeForce() and analyticCoulombForce() actually just returned the derivatives of the functions (i.e. the negation was missing). These functions are used to seed the tabulated potentials.
  2. Because of 1) the tabulated values in dUFull_ were, as the variable name suggested, the derivatives and not the force, but the PairPotential::force() function returns the actual values without a negation, and so was returning the derivatives and not the force.
  3. All of the ForceKernel::forcesWith(out)Mim functions correctly calculated the vectors the forces were acting on as i -> j as written above, but added to atom i and subtracted from atom j, 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.

@trisyoungs trisyoungs changed the base branch from develop to separate-forms April 19, 2024 10:29
Base automatically changed from separate-forms to develop May 7, 2024 08:44
@trisyoungs trisyoungs marked this pull request as ready for review May 7, 2024 08:45
@trisyoungs trisyoungs force-pushed the clarify-forces-and-derivatives branch from 9cc4082 to b573b95 Compare May 7, 2024 12:20
@trisyoungs trisyoungs merged commit 7d0de7e into develop May 9, 2024
10 checks passed
@trisyoungs trisyoungs deleted the clarify-forces-and-derivatives branch May 9, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants