theochem
Speed up gradient and hessian computation Closes #99
Steps
- [x] Write a good description of what the PR does.
- [x] Add tests for each unit of code added (e.g. function, class)
- [x] Update documentation
- [x] Squash commits that can be grouped together
- [ ] Rebase onto master
Description
New code to speed up hessian and laplacian calculations. Two main modifications have been made:
-New gradient, laplacian and hessian calling functions: Old functions computed gradient, laplacian and hessian matrices in a general form that calculated several derivative orders more than once and in some cases symmetry was not used. New functions are implemented to take advantage of this and speed up the code.
-New functions with explicitly coded first and second derivatives: 1st and 2nd derivative equations are coded explicitly in _deriv.py as the main goal was to speed up laplacian and hessian calculations, where the highest order for derivatives is 2. In _deriv.py 3 new functions were added, _eval_first_second_order_deriv_contractions() with the main code to deal with the calculation and gbasis organization, and _first_derivative() and _second_derivative() where explicit formulas for 1st and 2nd from gbasis notes are coded.
New argument deriv_type: A new argument deriv_type was introduced to be able to choose between _eval_deriv_contractions()- general formula for derivation of contractions, and _eval_first_second_order_deriv_contractions() - direct formula for 1st and 2nd derivatives.
New tests for _eval_first_second_order_deriv_contractions()-direct formula for 1st and 2nd derivatives in test_density_direct.py. It is the same test module as test_density.py but with deriv_type argument set equal to direct.
Type of Changes
| Type | |
|---|---|
| ✓ | :bug: Bug fix |
| ✓ | :sparkles: New feature |
| ✓ | :hammer: Refactoring |
| ✓ | :scroll: Docs |
Related Issue
Also, the linters QA fails which can be checked with running
pip install --user tox
tox -e qa
from the package home directory. I think you can get most of them by using black -l 100 (if not installed pip install --user black)
Again, I don't think they're deal-breakers, mostly tedious work that can be passed onto others maybe (or never get done even). Still, I think someone else more involved should make the final call
Thanks @kimt33 for your review and sorry for the late reply. I tried to fix all the issues related to "format" I would say and, if I am not wrong, all of them were detected by flake8. Then I have a recurrent issue reported by pylint Too many local variables (21/15) (too-many-locals). This will take me more time as it means I should look again at the code closely. This is a little bit my fault as I tend to code this way, I don't know if it could be a problem so let me know if you think so and want me to take care.
No worries! I don't think there is any rush with this repo. I'm also replying late haha
You don't need to worry about many of the linter errors, especially things like too-many-locals. It normally functions as a discretionary thing to keep some consistency with style. Usually whoever is managing the repository would make the final call or even make corrections, otherwise, it's more or less up to the person making the PR.
Just to be a bit technical, in the cases where PR fails the QA test set in place, you can usually get away with a separate commit which just adds a comment (something like pylint: disable=too-many-locals) with an explanation as to why that check was disabled. If the admin has a problem with it, it'd be easier to fix this way.
No need to worry about anything here, since the QA doesn't seem to be working anyways. Thanks again for working on this.