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

Fix to the issue #338 and issue #335 #339

Merged
merged 2 commits into from
Apr 12, 2021
Merged

Fix to the issue #338 and issue #335 #339

merged 2 commits into from
Apr 12, 2021

Conversation

msoroush
Copy link
Collaborator

@msoroush msoroush commented Apr 5, 2021

Fix the error in calculating intra-nonbonded energies for polar system, where rCutCoulomb is different from rCut. Now we only Use the LJ cutoff (rCut) do calculate the intra-nonbonded energies.
Increase the tolerance to 10 degree. If sum Angles +/- 10 was close to 360, we keep the angles rigid for cyclic molecules.

Increase the tolerance to 10 degree. If sum Angles +/- 10 was close to 360,
we keep the angles rigid for cyclic molecules.
@GregorySchwing
Copy link
Collaborator

Should we be concerned with electrostatic cutoffs for intra-terms? The inter terms have an overlap array which checks

if(forcefield.rCutLowSq < distSq) and the CalcCoulomb method checks if**(distSq < forcefield.rCutCoulombSq)**

But, there is no equivalent for intra terms. The CalcCoulombAdd_1_4, and CalcCoulombAdd_1_3 methods should have
if(forcefield.rCutCoulombSq[b] < distSq)
return 0.0;

Also, we don't check for if(forcefield.rCutLowSq < distSq) for intra-interactions, which seems like it could occur.

What is the reason for this?

@msoroush
Copy link
Collaborator Author

msoroush commented Apr 6, 2021

Should we be concerned with electrostatic cutoffs for intra-terms? The inter terms have an overlap array which checks

if(forcefield.rCutLowSq < distSq) and the CalcCoulomb method checks if**(distSq < forcefield.rCutCoulombSq)** But, there is no equivalent for intra terms.

We check forcefield.rCutLowSq < distSq to reject the move that causes overlap between atoms, this overlap only needs to be check for inter energy, not intra energy. When only need to calculate inter-nonbonded coulomb if they are within forcefield.rCutCoulombSq. This is because of ewald summation method. For intra-nonbonded coulomb, we dont use ewlad, so no need to compare with forcefield.rCutCoulombSq.

The CalcCoulombAdd_1_4, and CalcCoulombAdd_1_3 methods should have
if(forcefield.rCutCoulombSq[b] < distSq)
return 0.0;

@jpotoff correct me if I am wrong, but for calculating intra-nonbonded energy, I am using only one cutoff value rCut. The issue was inconsistency in using cutoff when we calculate intra-nonbonded energy in startup (uses rCut) and CBMC (uses max (rCut, rCutCoulomb)). In this pull request, we are only using rCut everywhere in code, to calculate intra-nonbonded energy.
@GregorySchwing similar to CalcAdd_1_4, no need to check the distance.

Also, we don't check for if(forcefield.rCutLowSq < distSq) for intra-interactions, which seems like it could occur.

We dont check for overlap, when we calculate intra-nonbonded energy, only in inter-nonbonded energy.

…es (elec and VdW) to avoid mistake in future implementations. All distance is now will be checked in force field class level.
@msoroush
Copy link
Collaborator Author

@GregorySchwing Can you review the latest change in this Pull request?

@GregorySchwing GregorySchwing merged commit f5bdcb3 into GOMC-WSU:development Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants