TinkerTools / tinker

Tinker: Software Tools for Molecular Design
https://dasher.wustl.edu/tinker/
Other
130 stars 61 forks source link

Debug compilation issue with MMFF #63

Closed NF-AMU closed 4 years ago

NF-AMU commented 4 years ago

Hello, When compiling Tinker8.7.2 in debug mode using gfortran version 9.3.0 (Ubuntu 9.3.0-10ubuntu2) and OPTFLAGS := -Og -g -ffpe-trap=invalid,zero,overflow -Wall -fcheck=all -fbacktrace, running the aspirin case available in the examples directory fails with: Program received signal SIGFPE: Floating-point exception - erroneous arithmetic operation.

Backtrace for this error:

0 0x7ff094a5fcd1 in ???

1 0x7ff094a5eea5 in ???

2 0x7ff09474520f in ???

3 0x55e9716deca3 in kvdw_

at /home/ferre/tinker/8.7.2/source_orig/kvdw.f:352

4 0x55e971338d19 in mechanic_

at /home/ferre/tinker/8.7.2/source_orig/mechanic.f:94

5 0x55e9711a42b0 in analyze

at /home/ferre/tinker/8.7.2/source_orig/analyze.f:54

6 0x55e9711a54c4 in main

at /home/ferre/tinker/8.7.2/source_orig/analyze.f:23

Using gdb shows that the error is precisely: 352 gik = (rad(i)-rad(k))/(rad(i)+rad(k)) which I confirms by printing both i and k values of the rad array. Both are 0.0d0, hence the forbidden division par 0.

Note that this error does not show up when normal optimization is achieved at compilation time.

Best regards, Nicolas Ferré, Aix-Marseille Université, France

jayponder commented 4 years ago

Hi, This is in the setup of the pairwise "radius" parameter matrix when using MMFF vdw. It's an easy fix since when both atomic radii are zero, the final matrix entry should just be zero and the calculation leading to the error is unnecessary. It just needs to be "safeguarded" in the code. I've just made this change to the Github "release" repo of Tinker 8.8, and it will appear on /dasher.wustl.edu/tinker in the next release there.

Thanks for using Tinker and for reporting this. I'm marking it fixed and closing the issue.

NF-AMU commented 4 years ago

Your correction works for this part of the code, thank you. Unfortunately, the same test case fails line 383: radmin is equal to 0.0 because of lines 371, 372.

NF-AMU commented 4 years ago

It will probably be a problem also in the 1-4 part of the same subroutine.

jayponder commented 4 years ago

Sorry. I've now put in the corresponding fixes for both rad and eps, and for both "standard" and 1-4 parameters. Let me know if that doesn't completely solve the issue.

NF-AMU commented 4 years ago

Thank you. There is still a couple of divisions by 0, lines 384 and 451 in which Nn can be 0.0.

jayponder commented 4 years ago

OK. I’ve fixed those as well, and I don’t see any other potential divide by zero cases in the MMFF vdw setup. These only occur because Tinker is filling pairwise parameter matrices for rad and eps that extend beyond the actual number of MMFF atom classes in the official MMFF parameters (in case users want to add more parameters via a key file, etc.). So it doesn’t affect any results using standard MMFF. I’m surprised this has not been reported before. But as you noted, it seems to only occur on Linux when compiled with debug flags.

On Jul 24, 2020, at 4:01 AM, NF-AMU notifications@github.com wrote:

Thank you. There is still a couple of divisions by 0, lines 384 and 451 in which Nn can be 0.0.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

-- Jay W. Ponder Phone: 1-314-935 4275 Chemistry, Campus Box 1134 Fax: 1-314-935 4481 Washington University in St. Louis One Brookings Drive Email: ponder@dasher.wustl.edu St. Louis, Missouri 63130 USA WWW: http://dasher.wustl.edu/

NF-AMU commented 4 years ago

Indeed, the final fix works, thank you. Actually, I sometimes need to compile Tinker in debug mode because I use it to include modifications and developments regarding QM/MM calculations, hopefully using any ff available in Tinker soon.