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

Libint2 one electron integrals #2388

Merged
merged 37 commits into from
Mar 11, 2022
Merged

Libint2 one electron integrals #2388

merged 37 commits into from
Mar 11, 2022

Conversation

andysim
Copy link
Member

@andysim andysim commented Dec 30, 2021

Description

Adds libint2 one electron integrals. A few integrals still use the hand-written code for now, but they will be addressed in subsequent PRs, as linked below.

Todos

New Features for Release Notes

  • Screening of one electron integrals to improve efficiency of PCM, EFP, and embedding methods
  • Parallelized PCM integral computations, which are typically rate-limiting for implicit solvent SCF

Checklist

Status

  • Ready for review
  • Ready for merge

@andysim andysim changed the title Libint2 one electron integrals [WIP Libint2 one electron integrals [WIP] Dec 30, 2021
@JonathonMisiewicz
Copy link
Contributor

JonathonMisiewicz commented Dec 31, 2021

This PR isn't marked "ready for review." Is there anything this needs from other devs?

@andysim
Copy link
Member Author

andysim commented Jan 4, 2022

I may need some help building a new Windows L2 library with the extra OEI hessians; the VM that I had set up is no longer available after the IT folks kindly sent a security patch that bricked my laptop. I should be able to get Parallels running again if needed, but would appreciate help from anyone with a Windows machine.

The failure in ADCC is something I see locally too; I get ImportError: /u/andysim/anaconda3/envs/psi4dev/lib//python3.9/site-packages/../.././libmkl_sequential.so.1: undefined symbol: mkl_lapack_xzlaswp_i4, so it looks like some kind of Conda dependency problem. If I try and build ADCC instead, I get problems finding libtensorlight, which I have installed via Conda. Any clues about this are welcome, before I head down the rabbithole.

@maxscheurer
Copy link
Contributor

Some of the adcc tests seem to work fine though? I will look into it ASAP.

@jturney
Copy link
Member

jturney commented Jan 4, 2022

I have a Windows box that I can try to build L2 on.

@andysim
Copy link
Member Author

andysim commented Jan 4, 2022

None of my ADCC tests are passing; all have the same problem. It's an old psi4dev Conda environment, so I should probably remake that first.

@loriab
Copy link
Member

loriab commented Jan 4, 2022

adcc/libtensorlight were among those that needed a rebuild late Nov when I moved the mkl pinning from >=2019.4,<2021 to >=2021.4,<2022. so a fresh local psi4-dev env is in order. azure should have been using new packages already, though.

@maxscheurer
Copy link
Contributor

azure should have been using new packages already, though.

Yes, that's what I meant. On Azure, only one adcc test fails...

Copy link
Member

@loriab loriab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, though I'd treat myself as a grey checkmark on this one. delightful delta LOC.

I'll run a tests w/threading on this locally and report back.

psi4/src/psi4/libcubeprop/csg.cc Outdated Show resolved Hide resolved
psi4/src/psi4/libmints/sointegral.cc Show resolved Hide resolved
psi4/src/psi4/scfgrad/response.cc Outdated Show resolved Hide resolved
psi4/src/psi4/scfgrad/response.cc Outdated Show resolved Hide resolved
psi4/src/psi4/libmints/electricfield.cc Outdated Show resolved Hide resolved
psi4/src/psi4/libmints/nabla.cc Show resolved Hide resolved
psi4/src/psi4/libmints/oeprop.cc Outdated Show resolved Hide resolved
This was referenced Jan 8, 2022
@andysim
Copy link
Member Author

andysim commented Jan 15, 2022

The latest CMake fix worked, so Linux is in good shape now. Before we can merge this, we'll need to make sure we have conda L2 set up correctly. We don't need the high angular momentum multipoles - I have a different strategy in mind for those. We will need the second derivatives of the one electron ints though. The lack of those hessian ints is what's causing the remaining failures on Windows.

Copy link
Contributor

@maxscheurer maxscheurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some spots where maybe #include "psi4/libmints/osrecur.h" is not needed anymore 😄

psi4/src/psi4/libmints/electricfield.h Show resolved Hide resolved
psi4/src/psi4/libmints/kinetic.h Show resolved Hide resolved
psi4/src/psi4/libmints/nabla.h Show resolved Hide resolved
psi4/src/psi4/libmints/overlap.h Show resolved Hide resolved
psi4/src/psi4/libmints/potentialint.h Outdated Show resolved Hide resolved
psi4/src/psi4/libmints/tracelessquadrupole.h Show resolved Hide resolved
@loriab loriab mentioned this pull request Feb 15, 2022
loriab and others added 7 commits February 15, 2022 00:35
Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>
Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>
Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>
Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>
@andysim andysim changed the title Libint2 one electron integrals [WIP] Libint2 one electron integrals Feb 17, 2022
Copy link
Contributor

@JonathonMisiewicz JonathonMisiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel a lot better about this, knowing that I can bug either Andy or Max on this PR if we have problems later. Please tend to the two changes I requested in this latest round, and again, reminder to coordinate with Lori on where to get L2 from, for test purposes.

Copy link
Contributor

@maxscheurer maxscheurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments! Once L2 conda deployment + tests are working, good to go! 🚀

Copy link
Contributor

@zachglick zachglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for all of the hard work! LGTM

Comment on lines 39 to 40

#define MAX(a, b) ((a) > (b) ? (a) : (b))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define MAX(a, b) ((a) > (b) ? (a) : (b))

Comment on lines 33 to 34

#define MAX(a, b) ((a) > (b) ? (a) : (b))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define MAX(a, b) ((a) > (b) ? (a) : (b))

Comment on lines 35 to 37

#define MAX(a, b) ((a) > (b) ? (a) : (b))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define MAX(a, b) ((a) > (b) ? (a) : (b))

@loriab loriab merged commit 9f7b865 into psi4:master Mar 11, 2022
@loriab loriab added the Libint2 label Mar 23, 2022
zachglick pushed a commit to zachglick/psi4 that referenced this pull request Apr 18, 2022
* Adds .gitattributes file to normalize line endings.

* Starting on libint2 overlaps

* Add devcontainer stuff

* Working on converting one-electron integrals to

* Full ctests passing at this point

* Fixes for Pytests.  All tests pass now.

* Remove buggy Pseudospectral derivs, GShell functions

* Make compute_pair private again

* Fix and parallelize PCM integrals

* Get correct l2 library when testing - to be reverted later

* Remove Git setup files

* Get L2 from the right repo

* Remove buffer call from one body ints

* Fix syntax in matrix header

* Correct for antisymmetric operators in onebody compute method

* Make sure CMake applies same standard to all L2 files

* Add Lori suggestions and docs, remove more physconst includes

* Use make_unique in one electron ints

* Use libint2 for relativistic potential ints

* Fix PCM with GCC

* Update azure-pipelines-windows.yml

* Update psi4/src/psi4/libmints/dipole.cc

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

* Update psi4/src/psi4/libmints/dipole.cc

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

* Update doc/sphinxman/source/prog_integrals.rst

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

* Update doc/sphinxman/source/prog_integrals.rst

* Update doc/sphinxman/source/prog_integrals.rst

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

* Update doc/sphinxman/source/prog_integrals.rst

* Use pairlist for gradient and hessian integrals

* Add more docs

* Fix test_mints pytest

* Apply changes requested in reviews

* Remove test of ADC with symmetry

* Small review comments

* Add Lori's suggested test fix

* Update meta.yaml

Co-authored-by: Justin Turney <justin.turney@gmail.com>
Co-authored-by: Lori A. Burns <lori.burns@gmail.com>
Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>
@adreasnow
Copy link

adreasnow commented May 21, 2022

Thank you so much for this guys! This has been a real sticking point for me for a lot of applications of Psi4.

@mfherbst mfherbst mentioned this pull request Nov 7, 2022
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Libint2 libmints For things that are wrong with libmints (but not wavefunction).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants