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

Consolidate clearing of eflags and vflags #1365

Merged
merged 9 commits into from Mar 25, 2019

Conversation

@martok
Copy link
Collaborator

martok commented Mar 13, 2019

Note: Purpose of the PR changed as per Comment #1

Summary

When applying flags at the start of a compute call, most pair styles only clear some of the flags. There is a function ev_unset that clears all flags, but that is rarely used.
The same pattern exists for fixes, impropers, angles, dihedrals, bonds and kspace styles.

This PR replaces the shared pattern with a central function that clears all flags.

Author(s)

Sebastian Hütter (OvGU)

Licensing

By submitting this pull request, I agree, that my contribution will be included in LAMMPS and redistributed under either the GNU General Public License version 2 (GPL v2) or the GNU Lesser General Public License version 2.1 (LGPL v2.1).

Backward Compatibility

No functional changes (hopefully!).
However, as a large number of files was changed that did not clear some flags, if at any point they were used erroneously, that might matter now.

Post Submission Checklist

Please check the fields below as they are completed after the pull request has been submitted. Delete lines that don't apply

  • The feature or features in this pull request is complete
  • Licensing information is complete
  • Corresponding author information is complete
  • The source code follows the LAMMPS formatting guidelines
  • Regression test suite including accelerators passes
@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 13, 2019

@martok pair->compute(0,0) is called quite frequently, e.g. during MD with fixed volume or minimization, when there is no thermo output or other access to energies. That not all pair styles call ev_unset() must be for historical reasons, i.e. the manual clearing of those flags must have been used until somebody, probably @sjplimp decided to consolidate it, but then did not apply it everywhere for reasons unknown. The difference, that I is that eflag_either is not zeroed, which doesn't seem to be used a lot.

So, if we are consolidating it now, we should be doing it now all the way. I am assigning @sjplimp to this PR so he can keep an eye on it and give further advice.

@martok

This comment has been minimized.

Copy link
Collaborator Author

martok commented Mar 13, 2019

Aye, eflags_either was the problem in this case.
I'll grep over the other pair styles make the same change, then.

Edit: Actually, almost nothing uses ev_clear() (only quip, reax/c and now meam/c do), all other pairstyles only set individual fields. This looks somewhat intentional?

OTOH, this would mean we'd get instances of this exact code segment everywhere:

 if (eflag || vflag) ev_setup(eflag,vflag);
  else ev_unset();

I'd prefer a function for that.

@martok martok changed the title USER-MEAMC: fix incomplete clearing of ev variables Consolidate clearing of eflags and vflags Mar 13, 2019

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 20, 2019

@sjplimp can you make looking at this a priority? i'd like to have this change, if approved, included quickly.

@timattox

This comment has been minimized.

Copy link
Collaborator

timattox commented Mar 21, 2019

Changes to USER-DPD look fine. I hope to try them out in my test suite tomorrow.

@sjplimp
Copy link
Contributor

sjplimp left a comment

looks fiine

@akohlmey

This comment has been minimized.

Copy link
Member

akohlmey commented Mar 25, 2019

@sjplimp your requested tweak is applied. please confirm, that this is what you asked for.

@sjplimp
Copy link
Contributor

sjplimp left a comment

yes, perfect

@akohlmey akohlmey merged commit 1bd47f0 into lammps:master Mar 25, 2019

6 checks passed

lammps/pull-requests/build-docs-pr head run ended
Details
lammps/pull-requests/cmake/cmake-serial-pr head run ended
Details
lammps/pull-requests/kokkos-omp-pr head run ended
Details
lammps/pull-requests/openmpi-pr head run ended
Details
lammps/pull-requests/serial-pr head run ended
Details
lammps/pull-requests/shlib-pr head run ended
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.