Project

General

Profile

Bug #961

SHAKE errors are reposrted incorrectly and may occationally go unnoticed

Added by Erik Marklund almost 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Category:
mdrun
Target version:
Affected version - extra info:
Affected version:
Difficulty:
uncategorized
Close

Description

A)
vec_shakef() calls cshake() and inspects the variable "error" to see if the constraints were applied successfully. error is supposed to hold the index in an array of constraints, so by looking at error one knows which constraint that caused the problem. This is problematic, since if the first constraint in the array (index 0) was unsuccessfully applied then error is 0 and everything looks ok:
error=0;
nconv=1;
for (nit=0; (nit<maxnit) && (nconv != 0) && (error == 0); nit++) {
nconv=0;
for(ll=0; (ll<ncon) && (error == 0); ll++) {

< snip >

if (rrpr < toler*mytol) 
error=ll;
< snip >
}
}

As seen, ll and error can be 0 when something goes wrong.

B)
When there's a SHAKE error the atoms and the constraint are not reported correctly to stderr. Currently we have the following:

else if (error != 0) 
{

< snip >

fprintf(stderr,"Inner product between old and new vector <= 0.0!\n" 
"constraint #%d atoms %u and %u\n",
error-1,iatom[3*(error-1)+1]+1,iatom[3*(error-1)+2]+1);
nit=0;
}

We report that constraint nr error-1 was bad, which is nonsense. Especially in the case where the first constraint was to blame. Consequently, we also report that atoms belonging to that wrongfully accused constraint are part of the problem.

SUGGESTED REMEDY: set error to the constraint index+1 in cshake() and crattle().

Associated revisions

Revision 7c611bab (diff)
Added by David van der Spoel almost 5 years ago

Fixes #961 - Shake reports wrong error

The constraint index reported by shake was off by 1.

Change-Id: I6f4d0c013b30f5963fb312b41fb87e90e6314619

History

#1 Updated by Erik Marklund almost 5 years ago

  • Assignee changed from Berk Hess to David van der Spoel

First of all, sorry for the formatting in the initial post. It was unintentional.

Secondly, I didn't mean to assign it to Berk. I changed the assignee to David, but I could also fix this myself if David's not up for it.

Also available in: Atom PDF