Project

General

Profile

Feature #1962

smarter box-size checking with the pull code

Added by Chris Neale over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
mdrun
Target version:
Difficulty:
uncategorized
Close

Description

src/gromacs/pulling/pull.c on line 520 in gromacs v5.1.2 has a check that the distance between pull groups not get larger than 0.49 times the box size. It would be nice if this check were a little smarter given the mdp options. For example, if pulling is only along z, then a system with large z but relatively small x and y might fail this test when there is no good reason for the run to be stopped. The change would be in the max_pull_distance2() function also in pull.c

Associated revisions

Revision 3dfdadb8 (diff)
Added by Berk Hess over 3 years ago

Relax pull PBC check

The check in the pull code for COM distances close to half the box
was to strict for directional pulling. Now dimensions orthogonal
to the pull vector are no longer checked. The check was actually
not strict enough for directional pulling along x or y in triclinic
units cells, but that is a corner case.
Furthermore, the direction-periodic hint is now only printed with
geometry direction.

Added tests for the maximum pull distance calcuation.

Fixes #1962.

Change-Id: I8e389ba3f0490ca67586fd10bdc9d71d9957ab45

History

#1 Updated by Semen Yesylevskyy over 3 years ago

I was just about submitting the same one! We have a long and thin system and it fails due to smallest dimension over Y while pulling is along Z only.

#2 Updated by Chris Neale over 3 years ago

Dear Semen:

my solution was to simply comment out the entire if statement near line 520 in pull.c so that this error never arises. Obviously not safe for general usage, but I convinced myself that for this particular run it would behave properly. It's an OK workaround until smarter checking is developed.

#3 Updated by Roland Schulz over 3 years ago

Do you have a specific suggestion for how to make the smarter check? Of course we love most, direct code submission to gerrit ;-).

#4 Updated by Chris Neale over 3 years ago

Let me take some more time to diagnose this. Looking again at max_pull_distance2(), it does seem to do what one would want (i.e., it excludes dimensions that are turned off from the test for distances that are too long). Furthermore, I made a test system with box dimensions 3x3x90 nm and two atoms separated by 30 nm along z with a pull only in z and the simulation did not complain. Unfortunately, I have deleted my system that gave the error message upon exit. It seems likely that I actually just did something wrong. Semen, what is your usage for the pull code .mdp parameters that gives this issue and how quickly do you get the message? I recall that I got it in less than a minute, but I'm having trouble reproducing it now.

#5 Updated by Chris Neale over 3 years ago

After further review, I withdraw the request. I must have made some other error. I think that the checking is actually pretty smart. It doesn't work when pull-coord1-vec = 0 0 1 and pull-coord1-dim = Y Y Y, but it does work when pull-coord1-dim = N N Y, so the user already has a workaround. Thank you, Chris.

#6 Updated by Semen Yesylevskyy over 3 years ago

I can't check if this workaround works for me now, but if it is this have to be clearly written in the manual since such fails due to the box size are usual and rather annoying.

#7 Updated by Semen Yesylevskyy over 3 years ago

Chris Neale wrote:

Semen, what is your usage for the pull code .mdp parameters that gives this issue and how quickly do you get the message? I recall that I got it in less than a minute, but I'm having trouble reproducing it now.

I can't reproduce it as well but for me it was complaining immediately.

#8 Updated by Gerrit Code Review Bot over 3 years ago

Gerrit received a related patchset '1' for Issue #1962.
Uploader: Berk Hess ()
Change-Id: I9e99c2b1b3f259a758e110c6d858c5704671b703
Gerrit URL: https://gerrit.gromacs.org/5954

#9 Updated by Mark Abraham over 3 years ago

  • Assignee set to Berk Hess
  • Target version changed from future to 2016

#10 Updated by Gerrit Code Review Bot over 3 years ago

Gerrit received a related patchset '1' for Issue #1962.
Uploader: Berk Hess ()
Change-Id: I8e389ba3f0490ca67586fd10bdc9d71d9957ab45
Gerrit URL: https://gerrit.gromacs.org/5971

#11 Updated by Berk Hess over 3 years ago

I just realized the extra, unused dims are not an issue at all. I think that before I thought that we would not always get the closest periodic image distance in dr, but we actually do.
I uploaded a proper fix at:
https://gerrit.gromacs.org/#/c/5971/

#12 Updated by Mark Abraham over 3 years ago

  • Status changed from New to Fix uploaded

#13 Updated by Berk Hess over 3 years ago

  • Status changed from Fix uploaded to Resolved

#14 Updated by Erik Lindahl over 3 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF