Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Series implementation of auxiliary gamma function #119

Merged
merged 14 commits into from
Oct 9, 2023
Merged

Series implementation of auxiliary gamma function #119

merged 14 commits into from
Oct 9, 2023

Conversation

hatemhelal
Copy link
Contributor

@hatemhelal hatemhelal commented Oct 6, 2023

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 =================================

@hatemhelal hatemhelal self-assigned this Oct 6, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@hatemhelal hatemhelal requested a review from awf October 9, 2023 10:16
notebooks/gammanu.ipynb Show resolved Hide resolved
notebooks/gammanu.ipynb Show resolved Hide resolved
notebooks/gammanu.ipynb Show resolved Hide resolved
notebooks/gammanu.ipynb Show resolved Hide resolved
notebooks/gammanu.ipynb Show resolved Hide resolved
pyscf_ipu/experimental/special.py Outdated Show resolved Hide resolved
test/test_experimental.py Outdated Show resolved Hide resolved
@hatemhelal
Copy link
Contributor Author

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
Copy link
Collaborator

awf commented Oct 9, 2023

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 hatemhelal changed the title Implementation of auxiliary gamma function Series implementation of auxiliary gamma function Oct 9, 2023
@hatemhelal
Copy link
Contributor Author

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.

@awf awf self-requested a review October 9, 2023 14:55
@hatemhelal hatemhelal merged commit 9ca8e13 into main Oct 9, 2023
4 checks passed
@hatemhelal hatemhelal deleted the gammanu branch October 9, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance bottleneck identified in lower incomplete gamma function
2 participants