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

nwchem restart opt-in #332

Closed loriab closed 2 years ago

loriab commented 2 years ago

Description

Modification of #330 such that you have to deliberately enable NWChem restarts.

Changelog description

Status

codecov[bot] commented 2 years ago

Codecov Report

Merging #332 (cfe2f63) into master (cd6d3e6) will not change coverage. The diff coverage is 100.00%.

WardLT commented 2 years ago

What are your thoughts against keeping it automatic? I can think of more reasons to be happy that NWChem picks up from where it left off (the default behavior) than not.

loriab commented 2 years ago

What are your thoughts against keeping it automatic? I can think of more reasons to be happy that NWChem picks up from where it left off (the default behavior) than not.

When I started running pytest repeatedly locally, various nwchem tests started to fail. That could be remedied by adding tmpdir, but it's a sudden change in behavior to need that. Given that there's testing difficulties, that previously we've denied that one should even need to know the scratch directory http://docs.qcarchive.molssi.org/projects/QCEngine/en/stable/single_compute.html?highlight=scratch_directory#faq, and that consequences for qcfractal are unprobed (I think?), I'm reluctant to switch one program to opt-out at short notice.

WardLT commented 2 years ago

I agree with the points you raise above. Let's merge this, and I'll continue working on the documentation for the NWChem harness so people know how to safely use restarts.