pymatgen icon indicating copy to clipboard operation
pymatgen copied to clipboard

Parsing the Fock matrix and eigenvalues from the QChem output file

Open sudarshanv01 opened this issue 3 years ago • 3 comments

Summary

I am not sure if this typically required output to be parsed from a QChem file, so please feel free to let me know if this is not of interest.

This PR includes changes to qchem/outputs.py and qchem/utils.py files. In case the Fock-matrix and eigenvalues are requested by the user (though the flags scf_final_print or scf_print), the modifications to outputs.py allows for parsing both these quantities.

Additional dependencies introduced (if any)

No additional dependencies.

TODO (if any)

If this parsing is of interest, other matrix quantities such as the coefficients of the MO can be parsed by the same function in qchem/utils.py. Tests need to be added as well.

Checklist

Before a pull request can be merged, the following items must be checked:

  • [x] Code is in the standard Python style. The easiest way to handle this is to run the following in the correct sequence on your local machine. Start with running black on your new code. This will automatically reformat your code to PEP8 conventions and removes most issues. Then run pycodestyle, followed by flake8.
  • [ ] Docstrings have been added in the Google docstring format. Run pydocstyle on your code.
  • [x] Type annotations are highly encouraged. Run mypy to type check your code.
  • [x] Tests have been added for any new functionality or bug fixes.
  • [ ] All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more efficient if you already fix most errors prior to submitting the PR. It is highly recommended that you use the pre-commit hook provided in the pymatgen repository. Simply cp pre-commit .git/hooks and a check will be run prior to allowing commits.

sudarshanv01 avatar Jun 29 '22 01:06 sudarshanv01

Thanks Sudarshan! Flagging @samblau for comment. I don't use these features so don't have much to say other than I think it's great when our IO classes support as many features of the underlying code as possible.

rkingsbury avatar Jun 30 '22 17:06 rkingsbury

Coverage Status

Coverage remained the same at 0.0% when pulling 15807dbaa49edeba884e4a02851cfaec005acc16 on sudarshanv01:qchem_io_eigenvals into c1f1e95d93a1fab758610aeb9a68d5711e1fce82 on materialsproject:master.

coveralls avatar Jun 30 '22 23:06 coveralls

Hi @samblau ! I have added tests for parsing the Fock matrix and eigenvalues through the latest commit. These tests have been added to test_outputs.py and test_sets.py. Pylint requested changes in parts of the code that I have not changed are highlighted through comments.

Note that this commit is needed to complete this PR on atomate (which fails tests because it doesn't have the right keys for a dict implemented through this PR).

sudarshanv01 avatar Jul 08 '22 18:07 sudarshanv01

Thanks @janosh for taking a look at this! It appears that all checks pass now.

sudarshanv01 avatar Nov 01 '22 15:11 sudarshanv01

Looks good to me, let's get this PR merged @mkhorton @shyuep

samblau avatar Nov 01 '22 16:11 samblau

Done :) thanks @sudarshanv01!

mkhorton avatar Nov 01 '22 16:11 mkhorton