QC-program keywords

[jthorton]

Description

Fixes #238 by adding a program-based keyword generator until we have a general interface to accept keywords.

Todos

Notable points that this PR has either accomplished or will accomplish.

• TODO 1

Questions

• should I also extend the qc task to accept keywords so the user can set them? Then the new function _get_program_keywords could add program-specific keywords needed from our experience that are missing from the provided dictionary.

Status

• Ready to go

[jthorton]

needs #235 for testing faliures

[Yoshanuikabundi]

Thanks for looking into this @jthorton! Can I just make sure I understand what's going on here?

We expose QCSubmit's QCSpec class to users to define the calculation they want to make, but then when defining the tasks we want to run we only take the method, basis, and program and everything else is ignored. This happens here (is default_qc_specs used anywhere else? My IDE couldn't find anything else).

We want to add the 'muted' verbosity keyword to fix #238, so we're adding some code to add that keyword at the end to the specification of the QC job as an interim fix until something better can be worked out.

Have I got that right? If that's right, then I think this is great. I might also add a note to the big refactor issue that we should move away from using QCSpec here, since we don't support most of its settings?

[jthorton]

Hi, @Yoshanuikabundi that's right!

Our main reason for using QCSpec is the validation it provides on the combination of method, basis, program and implicit solvent but a few other fields are redundant like the spec name or description. Ideally, I think we would offer a base model in qcsubmit which still has this validation but with these redundant terms removed. We might also consider splitting the specification classes to offer even better validation or maybe even program-specific validation

The main features we will want here are validation of the method, basis and program combination along with the ability to pass keywords and possibly the validation of implicit solvent settings.

[Yoshanuikabundi]

This looks good! I'd like to add tests as we add new features, so I've written one in #244 - It's a PR to this PR. If you like it (after any tweaks you wanna make), could you merge it in here and make sure CI passes before merging this? Thanks @jthorton!

Also - do you know if its possible for the keyword to be set incorrectly in the input and then not get passed on to XTB without raising an error? Or do you know a way to test that the keyword was correctly set in tests?

[jthorton]

Thanks for adding the test.

do you know if its possible for the keyword to be set incorrectly in the input and then not get passed on to XTB without raising an error?

With most keywords, if it is misspelt it just won't be set by the wrapper around the program, and errors will only be raised if the value is incorrect in most cases. This is why we tried to build some validation into the QCSpec class to catch these before run time and would be another point in favour of more specialised program-specific keyword validation.

With regard to testing this case, it would make sense to check that the output is empty from each single point calculation to confirm the keyword was correct and accepted however the result is currently striped by the worker here to make the results objects smaller. The test does check that the keyword was inserted though which in this case is the best we can do I would say.

[codecov]

Codecov Report

Merging #239 (848100f) into main (ac5dd88) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files