openmopac / mopac

Molecular Orbital PACkage
http://openmopac.net
GNU Lesser General Public License v3.0
122 stars 32 forks source link

Fix erratic printing of GRADIENT NORM #71

Closed godotalgorithm closed 2 years ago

godotalgorithm commented 2 years ago

I've identified the cause and fixed #67. As expected, there was logic suppressing a print statement if the printed quantity was smaller than a threshold value, which is sensitive to rounding details that can change between machines. I have removed this instance, and I would ideally like to remove this type of behavior as it is identified. It's a bit presumptuous to assume that a user don't want to know an output when its value is zero, and I just don't want to deal with these complications to the testing process in any other way than removal (unless there is a very good reason for it).

Status

codecov-commenter commented 2 years ago

Codecov Report

Merging #71 (23fd320) into main (d136be2) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #71   +/-   ##
=======================================
  Coverage   68.23%   68.23%           
=======================================
  Files         330      330           
  Lines       71706    71706           
=======================================
  Hits        48932    48932           
  Misses      22774    22774           
Impacted Files Coverage Δ
src/output/writmo.F90 69.73% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d136be2...23fd320. Read the comment docs.

godotalgorithm commented 2 years ago

Actually, this instance is a bit messier than I first realized. Some of the logic detecting whether or not energy gradients have been calculated at all is relying on a numerical check of the overall gradient norm. Without more careful consideration, the most responsible thing to do for now is to replace the small numerical tolerance in the test with a strict zero comparison. It is extremely likely, but not strictly guaranteed that the numerical gradient will be exactly zero only when the gradient hasn't been computed at all (as it is initialized to exactly zero before being replaced by a computed value).

@susilehtola, you'll have to check if this fix is actually correcting your erratic test results, because this quick fix isn't guaranteed to do so.