Project

General

Profile

Bug #1898

[ cmaptypes ] silently corrupts entries when more than one spaces are inserted between atomtypes

Added by Shun Sakuraba over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
preprocessing (pdb2gmx,grompp)
Target version:
-
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

Current [ cmaptypes ] parser process [ cmaptypes ] entries assuming there is only a single space between atom types. Adding spaces between atomtypes silently corrupts cmap entries in grompp parsing.

Steps to reproduce:

% diff -u space1.mdp space2.mdp
% diff -u cmap_space1.itp cmap_space2.itp
% gmx grompp -f space1.mdp -o space1.tpr
% gmx grompp -f space2.mdp -o space2.tpr
% gmx mdrun -deffnm space1
% gmx mdrun -deffnm space2

Note gmx check reports there are no differences in tpr (because it doesn't compare inside cmaptype entries.)

Current behaviour (as of 5.0.7):
  • space1.log and space2.log show different CMAP energies.
    (3.17414e-01 kJ/mol for space1 and 1.42657e+01 kJ/mol for space2)
Expected behaviour:
  • Two values are equal.

Corresponding source codes:
In push_cmaptype() in src/gromacs/gmxpreprocess/toppush.c, the character position in first entry in line is calculated as:

    for (i = 0; i < nn; i++)
    {
        start += (int)strlen(alc[i]);
    }

    // (snip)

    start = start + nn -1;

where nn is the number of entries (=8). This however does not cover cases that multiple spaces are inserted between atomtypes. I believe "%n" should be used in sscanf() call to catch the correct starting point.

reproduce.tgz (21.1 KB) reproduce.tgz Shun Sakuraba, 02/05/2016 08:25 AM

Associated revisions

Revision 0098ea6d (diff)
Added by Berk Hess over 4 years ago

Fix incorrect cmap parsing

The cmap parsing in grompp expected exactly one space between
atomtypes and would silently corrupt the cmap parameters when more
than one space was present.
Also added cmap comparison to gmx check.

Fixes #1898.

Change-Id: If5911ce78bdec2a15dbcda7c2437dd6cae86c0ae

Revision b53b2f68
Added by Berk Hess over 4 years ago

Fix incorrect cmap parsing

The cmap parsing in grompp expected exactly one space between
atomtypes and would silently corrupt the cmap parameters when more
than one space was present.

Also added cmap comparison to gmx check.

Fixes #1898.

Change-Id: If5911ce78bdec2a15dbcda7c2437dd6cae86c0ae

History

#1 Updated by Gerrit Code Review Bot over 4 years ago

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

#2 Updated by Berk Hess over 4 years ago

  • Status changed from New to Fix uploaded
  • Assignee set to Berk Hess
  • Priority changed from Low to High

Since this silently corrupts parameters and produces energies within the same range, this is a high priority issue.
I pushed a fix to gerrit for release-5-1, but we might want this to be fixed in release-5-0.

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

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

#4 Updated by Berk Hess over 4 years ago

  • Status changed from Fix uploaded to Resolved

#5 Updated by Berk Hess over 4 years ago

#6 Updated by Erik Lindahl over 4 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF