Project

General

Profile

Bug #2705

Bug in shell code minimization

Added by Berk Hess about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
preprocessing (pdb2gmx,grompp)
Target version:
Affected version - extra info:
all previous 2018 versions
Affected version:
Difficulty:
uncategorized
Close

Description

The shell and flexible constraint minimization used the same pointer for the trial and minimum configuration. Thus minimization is unaffected when no step is rejected until the minimum is reached. But when a step is rejected, many more iterations might be needed because the coordinates are not reset to those before the trial step.

Associated revisions

Revision 93109ddf (diff)
Added by Mark Abraham about 1 year ago

Introduce ArrayRefWithPadding

Modules need to access views of memory with and without padding. This
class can be constructed from a PaddedVector, and provides the
capacity to hide the underlying padded container (and allocator),
while being able to obtain ArrayRef with and without padding.

Also updated wording of docs for ArrayRef, and attribution

Fixed issue with shell code using the same pointer for trial and
minimum configuration.

Fixes #2642
Fixes #2705

Change-Id: I45197215342ae011298abc310b877c34f8fab88b

Revision 5749eb1c (diff)
Added by Mark Abraham about 1 year ago

Fix inefficient shell minimization

In 5268b73d6cd, a bug was introduced when a shallow copy was used
instead of the former deep copy. However, in the original code some
pointers were used as "short-cuts", which is not neccessary. Those
pointers have been removed, and now the shell-code position vectors
are fully initialized at allocation time.

In order to get reproducible normal modes the delta x used for
computing derivatives was reduced.

Fixes #2705

Change-Id: Iad8a0b0793304e2c10d9abf3a180cfe9bdc85c4c

History

#1 Updated by Mark Abraham about 1 year ago

  • Assignee set to Mark Abraham
  • Affected version - extra info set to all previous 2018 versions

I believe I broke this in 5268b73d6cd2966426540a, which was before 2018 was released. There's a fix contained in https://gerrit.gromacs.org/#/c/8595/ that we should back port

#2 Updated by Berk Hess about 1 year ago

My guess is that is effectively only a performance issue, not a correctness issue.

#3 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #2705.
Uploader: Mark Abraham ()
Change-Id: gromacs~release-2019~I45197215342ae011298abc310b877c34f8fab88b
Gerrit URL: https://gerrit.gromacs.org/8595

#4 Updated by Mark Abraham about 1 year ago

  • Status changed from In Progress to Resolved

#5 Updated by Paul Bauer about 1 year ago

  • Status changed from Resolved to Closed

#6 Updated by Mark Abraham about 1 year ago

  • Status changed from Closed to Accepted

Not yet back ported

#7 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #2705.
Uploader: Mark Abraham ()
Change-Id: gromacs~release-2018~Iad8a0b0793304e2c10d9abf3a180cfe9bdc85c4c
Gerrit URL: https://gerrit.gromacs.org/8676

#8 Updated by Mark Abraham about 1 year ago

  • Status changed from Accepted to Fix uploaded

#9 Updated by Gerrit Code Review Bot about 1 year ago

Gerrit received a related patchset '1' for Issue #2705.
Uploader: David van der Spoel ()
Change-Id: regressiontests~release-2018~If4352edfc75b305dd0ce41bb741c289ff649e2da
Gerrit URL: https://gerrit.gromacs.org/8678

#10 Updated by Paul Bauer about 1 year ago

  • Target version changed from 2018.4 to 2018.5

This looks like it will involve more bug fixing, bumped to next patch.

#11 Updated by David van der Spoel about 1 year ago

Looking into this a bit more. We have forces on the shell that are very small, 1e-3 kJ/mol nm. According to Hooke's law F = k Dx, so we compute Dx, the change in position to minimize the energy (or get zero force) as
Dx = F / k
Since k in this case ~ 500 000 kJ/mol nm^2 we have Dx = 2e-10 nm which leads to precision issues.

Then this is used for the numerical derivation of the Hessian using df/dx = (f(x+delta)-f(x-delta))/(2 delta).

Will investigate some more.

#12 Updated by Mark Abraham about 1 year ago

  • Status changed from Fix uploaded to Resolved

#13 Updated by Paul Bauer about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF