step-size scaling in lbfgs doesn't work as intended
When converting minimize.c to C++, clang-static-analyzer notices that stepsize gets scaled by golden ratio numbers around line 1960, but then stepsize is unilaterally set to the value of c around line 2142. Removing the latter suppresses the error from the static analyzer, and seems consistent with trying to do scaling of step size. See clang-static-analysis errors at https://gerrit.gromacs.org/#/c/4010/. Thoughts?
Convert remaining callers of do_force to C++
Major callers are converted in other patches.
Removed unused variables, used std::min/max, named constants, put
maximum length on sscanf floats.
Introduced an instance of bMaster. Initialized gstat to and later
checked for NULL before use. These help static analysis reason
correctly. Previous code functioned correctly in practice, but correct
analysis would require cross-function analysis that cr, when passed as
non-const, is not actually modified. Fixed a warning about stepsize
not being used in L-BFGS, and added more comments to this algorithm
while editing the code to help us understand what it does.
The predict_shells() function has been modified to use the
gmx_mtop_atomnr_to_atom() call for case 2; the previous version would
use an uninitialized pointer.
#2 Updated by Erik Lindahl over 5 years ago
- Status changed from New to Fix uploaded
It was slightly more complicated than that. The stepsize variable was used to indicate the step taken (which is used in the correction factor application a few lines further down), but I've introduced a new var for this instead.
Tested to work on 2-3 different systems - it produces the same energy minima as before.
#3 Updated by Mark Abraham over 5 years ago
But the static analyzer points out that stepsize is now unused, rather merely over-written... http://jenkins.gromacs.org/job/clang-static-analyzer_Gerrit_master/2377/javadoc/... and there is some new unrelated issue in shellfc, also.