MolSSI / QCEngine

Quantum chemistry program executor and IO standardizer (QCSchema).
https://molssi.github.io/QCEngine/
BSD 3-Clause "New" or "Revised" License
163 stars 79 forks source link

Properly handle compute errors raised in berny optimization procedure #301

Closed coltonbh closed 2 years ago

coltonbh commented 3 years ago

Description

Closes #300 .

Changelog description

Handle gradient computation errors correctly inside of BernyProcedure.compute() method

Note on decisions

Open Questions

Status

coltonbh commented 3 years ago

Another one for ya @loriab :)

codecov[bot] commented 3 years ago

Codecov Report

Merging #301 (0490362) into master (cf5fdc5) will decrease coverage by 10.96%. The diff coverage is 96.15%.

coltonbh commented 2 years ago

@loriab wanted to check in on this now that I'm back to developing a bit. Old PR--possible to merge in? This fixes basic error handling that was incorrect inside the berny procedure.

Thanks!

loriab commented 2 years ago

Is it necessary to declare types in the function signature using the "" employed previously in this method? Seems more canonical to declare return types without "" values unless they are the class being defined in a https://github.com/classmethod. I have followed the convention already in the code but would consider updating it to remove the "" if allowed.

The "" types in function signatures are a legacy of (1) mypy being fired up for these repos only every year or two and (2) Sphinx docs not being able to do anything with signature types at the time of documentation. Without quick feedback on typing right/wrong, it was easiest to use the harmless strings. Now that #362 modernizes docs to use types from signature, I'm all for starting to convert to non-"" types. Once docs links are clean enough, we can turn on warnings-to-errors and nitpicky so that the docs build does some type checks (at least that imports work).