Project

General

Profile

Bug #700

Insufficient checks for input parameters related to wall options

Added by Jaanus Karo about 6 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Low
Assignee:
-
Category:
-
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

Defining walls around a simulation box needs to define atom type names for each wall (wall_atomtype). It's a string and therefore easy to mix up (as I did, mixing a name of atom and a molecule).
Currently there is no special control logic for wall options available and the grompp program generates happily a topol.tpr file, which contains this:

wall_atomtype[0]     = -12345
wall_atomtype[1]     = -12345

Now, running the mdrun program with the topol.tpr, generated above, a core dump is saved.

The situation is described more in detail here:
  • as the structure gpp_atomtype_t does not contain an accidentally configured wrong atom type name, the method get_atomtype_type returns NOTSET (-12345) in the file src/kernel/gpp_atomtype.c:76
  • without any further control, the structure t_inputrec gets the value -12345 in the file src/kernel/grompp.c:855 (ir->wall_atomtype[i] = get_atomtype_type(opts->wall_atomtype[i],at)). This is the location where I would appreciate at least some warnings about the situation.
  • now, in the mdrun code, an array "ntw" is initialized with these values - NOTSET; src/mdlib/wall.c:121
  • and finally, touching the array "nbfp" outside the boundaries, the core dump happens at line src/mdlib/wall.c:170 (Cd = nbfp[ntw[w]+2*at];)

Associated revisions

Revision 286575f0 (diff)
Added by Mark Abraham about 4 years ago

Added check for valid wall_atomtype in .mdp

Otherwise, mdrun would dump core when the garbage got dereferenced.

Note that this does not need to be merged into release-4-6, because
c7a82654 already fixes it.

Fixes #700

Change-Id: I39f440595f7b112d4f01b5e32218ec45a2a97370

History

#1 Updated by Teemu Murtola about 6 years ago

  • Tracker changed from Feature to Bug
  • Subject changed from Additional control for input parameters related to wall options to Insufficient checks for input parameters related to wall options

This is clearly a bug, so marked as such instead of a feature request. Also changed the title to match.

#2 Updated by Janne Blomqvist about 5 years ago

Also, when using user-generated potential tables for wall interactions (wall_type = table), then wall_atomtype should not be necessary. Currently it's necessary to set wall_atomtype to some existing atom types, even though AFAICS they are unused when wall_type == table.

#3 Updated by Rossen Apostolov almost 5 years ago

  • Target version set to 4.5.6

#4 Updated by Mark Abraham about 4 years ago

  • Status changed from New to Closed

Michael Shirts added a warning for this in c7a82654, which I have back-ported to release-4-5-patches in https://gerrit.gromacs.org/#/c/2054/, so I think the main issue is resolved.

On Janne's point, the tables define the functional form, but they do not encode (e.g.) the LJ parameters, which are still looked up from the atom types (per do_walls() in src/mdlib/wall.c). Happy to be proved wrong by some .tpr files, though ;-)

Also available in: Atom PDF