graphcore-research / pyscf-ipu

PySCF on IPU
https://github.com/graphcore-research/pyscf-ipu#readme
Apache License 2.0
42 stars 2 forks source link

Series implementation of auxiliary gamma function #119

Closed hatemhelal closed 1 year ago

hatemhelal commented 1 year ago

This PR adds another implementation gammanu to make use the series expansion of the lower incomplete gamma function. This is derived in the notebook included in this PR.

Closes #116 which motivated this journey into obscure numerical methods.

Using the command pytest --durations=8 will report the 8 slowest testpoints:

CPU tests with gammanu = gammanu_gamma

================================================== slowest 8 durations ==================================================
39.20s call     test/test_experimental.py::test_water_eri[True]
34.44s call     test/test_experimental.py::test_water_eri[False]
7.30s call     test/test_experimental.py::test_eri
5.68s call     test/test_experimental.py::test_eri_basis
4.15s call     test/test_experimental.py::test_gto[6-31+g]
3.30s call     test/test_experimental.py::test_gto[sto-3g]
3.04s call     test/test_experimental.py::test_nuclear
2.29s call     test/test_experimental.py::test_water_nuclear
=========================== 22 passed, 4 skipped, 1 xfailed, 5 warnings in 112.22s (0:01:52) ============================

CPU tests with gammanu = gammanu_series

================================================== slowest 8 durations ==================================================
10.05s call     test/test_experimental.py::test_eri
5.93s call     test/test_experimental.py::test_water_eri[True]
5.23s call     test/test_experimental.py::test_nuclear
4.84s call     test/test_experimental.py::test_eri_basis
4.17s call     test/test_experimental.py::test_gto[6-31+g]
3.47s call     test/test_experimental.py::test_gto[sto-3g]
2.41s call     test/test_experimental.py::test_water_nuclear
2.05s call     test/test_experimental.py::test_kinetic
================================= 22 passed, 4 skipped, 1 xfailed, 5 warnings in 50.32s =================================
review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

hatemhelal commented 1 year ago

I didn't spot this before but it looks like from the test timing report copied in the PR summary that the nuclear attraction integrals get slower with the switch to the series implementation of gammanu

awf commented 1 year ago

I didn't spot this before but it looks like from the test timing report copied in the PR summary that the nuclear attraction integrals get slower with the switch to the series implementation of gammanu

Interesting, and what happened to water[False]?

Still net goodness for CPU-based CI, and we know IPU will require some additional work.

hatemhelal commented 1 year ago

what happened to water[False]?

I think this is caching / jit helping since we effectively compute the same matrix elements twice. For example, selecting just that test with -k test_water_eri we see:

================================================== slowest 8 durations ==================================================
6.41s call     test/test_experimental.py::test_water_eri[True]
1.94s call     test/test_experimental.py::test_water_eri[False]

and -k test_water_eri[False] runs in around 7 sec since it doesn't benefit from the True case running ahead of it.