Skip to content

Created variable and method for storing and retrieving the gradients … #3087

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Dec 3, 2023
Merged

Created variable and method for storing and retrieving the gradients … #3087

merged 9 commits into from
Dec 3, 2023

Conversation

johnppederson
Copy link
Contributor

…on the external point charges of the ExternalPotential.

Description

Provides a way to get the gradient of the potential between a Wavefunction object and an ExternalPotential object on the external point charges. These gradients are collected and stored in a protected SharedMatrix object of the ExternalPotnetial during the ExternalPotential.computePotentialGradients() routine, and they are accessible through a ExternalPotential.gradient() method which is bound to a corresponding method in the Python API.

User API & Changelog headlines

  • Given a Wavefunction object with an ExternalPotential for which a gradient call has been made, the corresponding gradient on the embedded point charges represented by the ExternalPotential can be retrieved by calling gradient() on the ExternalField

Checklist

Status

  • Ready for review
  • Ready for merge

Pederson and others added 2 commits November 27, 2023 15:41
loriab
loriab reviewed Nov 27, 2023
View reviewed changes
@loriab loriab added this to the Psi4 1.9 milestone Nov 28, 2023
@loriab
Copy link
Member

loriab commented Nov 29, 2023

Don't worry about the Eco/Linux error -- it's #3088 (comment)

loriab
loriab reviewed Nov 29, 2023
View reviewed changes
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.

Thanks for the name change. It'd be good to have a test case, too. Do you have one in mind or do you want to add to tests/extern*/input.dat. I can advise on the next steps after.

johnppederson and others added 4 commits November 29, 2023 10:39
@johnppederson @loriab
Update psi4/src/psi4/libmints/extern.h
9ae4134 
Co-authored-by: Lori A. Burns <lori.burns@gmail.com>
@johnppederson @loriab
Update psi4/src/psi4/libmints/extern.cc
ddafac8 
Co-authored-by: Lori A. Burns <lori.burns@gmail.com>
@johnppederson @loriab
Update psi4/src/export_mints.cc
132680f 
Co-authored-by: Lori A. Burns <lori.burns@gmail.com>
@johnppederson
Merge branch 'master' into external-gradient
ac82817
@johnppederson
Copy link
Contributor Author

johnppederson commented Nov 29, 2023 •
edited
Loading

To test this feature, I have been comparing the gradient_on_charges with the gradient_on_atoms, since they should sum up to zero along each coordinate. It's not currently possible to get the gradient_on_atoms directly from the ExternalPotential, so I have been testing this locally by subtracting the gradient of the wfn (SCF in the presence of an ExternalPotential) with the ExternalPotential set to None from the gradient of the wfn with the ExternalPotential. I can add my local test to the test suite or I can write it into an existing test; which would make more sense?

@loriab
Copy link
Member

loriab commented Nov 30, 2023

To test this feature, I have been comparing the gradient_on_charges with the gradient_on_atoms, since they should sum up to zero along each coordinate. It's not currently possible to get the gradient_on_atoms directly from the ExternalPotential, so I have been testing this locally by subtracting the gradient of the wfn (SCF in the presence of an ExternalPotential) with the ExternalPotential set to None from the gradient of the wfn with the ExternalPotential. I can add my local test to the test suite or I can write it into an existing test; which would make more sense?

I see what you mean -- you'd need SCFDeriv::gradients_ to be exported to get at the gradient_on_atoms array. If that'd be generally useful, we could do it. But if it's just for testing, your procedure sounds good. Either way on new test or adding that extra computation and check to an existing extern* test. If you have a different molecule (we're stuck on water and N2), it might be nice to have a new test for variety. If you do do a new test, the procedure is to make a new dir in tests/, copy over the CMakeLists.txt, test_input.py, and input.dat from an existing extern test. Edit the name and the input.dat contents, and register the test in tests/CMakeLists.txt.

@johnppederson
Copy link
Contributor Author

Exporting SCFDeriv::gradients_ might be a useful feature to decompose the gradient (and force) contributions on the QM atoms, but for the test on gradient_on_charges, I have opted to use existing machinery in Psi4 in a new test directory, extern4.

loriab
loriab approved these changes Dec 3, 2023
View reviewed changes
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, thanks for the contribution!

@loriab loriab added this pull request to the merge queue Dec 3, 2023
Merged via the queue into psi4:master with commit f2246eb Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loriab loriab loriab approved these changes

@davpoolechem davpoolechem davpoolechem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Psi4 1.9
Development

Successfully merging this pull request may close these issues.
3 participants
@johnppederson @loriab @davpoolechem