MolSSI / QCEngine

Quantum chemistry program executor and IO standardizer (QCSchema).

https://molssi.github.io/QCEngine/

BSD 3-Clause "New" or "Revised" License

163 stars 78 forks source link

mopac ci lane. pin to qcel 0.25 #372

Closed loriab closed 2 years ago

loriab commented 2 years ago

Description

Changelog description

[x] add mopac testing to CI since @awvwgk kindly packaged it
- forgiving the electonic energy key seems ok since it was dropped here https://github.com/openmopac/mopac/commit/d6bdb50bcf74cef84e469097601a4e4944926bee#diff-06ee723679a49e30f3287e5ace72eb84e2e7b0703fb00be2c2de151cfb04e26cL422
- not too sure about other test loosening
[x] upcoming changes make it safer to pin tighter to qcel

EDIT: closes #373

Status

[x] Code base linted
[x] Ready to go

codecov[bot] commented 2 years ago

Codecov Report

Merging #372 (7ed7012) into master (f726364) will increase coverage by 14.79%. The diff coverage is 100.00%.

loriab commented 2 years ago

@godotalgorithm, any apprehensions about the test changes?

godotalgorithm commented 2 years ago

This looks ok to me - you are now getting MOPAC from conda-forge (which I will be actively maintaining), some reference numbers have been changed, and the "electronic energy" was removed.

The "electronic energy" has been confusing people (it is NOT a total electronic energy for the molecule), so it has been removed from the default output (it is still accessible with a keyword) and I'd prefer not to emphasize it. The forces produced by MOPAC are derivatives of the "heat of formation", so that is the primary energetic quantity of relevance.

The reference numbers (they seem like geometric data for water?) have shifted a bit, but that isn't surprising of an optimized geometry as MOPAC's default geometric convergence isn't that tight and a lot of small changes have happened behind the scenes in the last few years to perturb these results.

awvwgk commented 2 years ago

Maybe also update the suggestion on how to install Mopac at

https://github.com/MolSSI/QCEngine/blob/43487205d114de4760ea64fd0fa46652d31eda0d/qcengine/programs/mopac.py#L45

loriab commented 2 years ago

This looks ok to me - you are now getting MOPAC from conda-forge (which I will be actively maintaining), some reference numbers have been changed, and the "electronic energy" was removed.

Great! I grouped mopac with the d3/d4 and geometry optimizers for testing, but if you ever find it would benefit from its own testing lane, that can be arranged.

The reference numbers (they seem like geometric data for water?) have shifted a bit, but that isn't surprising of an optimized geometry as MOPAC's default geometric convergence isn't that tight and a lot of small changes have happened behind the scenes in the last few years to perturb these results.

Thanks for the confirmation that accumulated small shifts likely, esp. when combined with geometric algorithms.