-
Notifications
You must be signed in to change notification settings - Fork 475
Fix EMBPOT Functionality in SCF and Add EMBPOT Gradients #3239
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
Conversation
loriab
left a comment
There was a problem hiding this 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 bugfix, the new deriv functionality, and the tidy validating test case.
|
|
||
| FILE* input = fopen("EMBPOT", "r"); | ||
| int npoints; | ||
| int statusvalue = fscanf(input, "%d", &npoints); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check the value of statusvalue to ensure the read in was successful. Should probably figure out in the value of npoints isn't something outlandish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added checks for the statusvalue return on fscanf calls to read the EMBPOT file header and body in both mintshelper.cc and hf.cc, and I've also added a check to see if the EMBPOT file size (based on npoints) exceeds the allocated memory.
|
Hi @johnppederson, I think this one is ready for a rebase to resolve conflicts and get CI passing again. Also, Jet has a couple comments (above). I can also handle this locally if you're busy. Sorry for the extreme delay. Since #3291 is finally in, your #3243 could be rebased, but that's for v1.11, so at your leisure. |
jturney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes!
|
merge_queue failed on test |
Description
This pull request fixes the functionality of the EMBPOT perturbation to the core Hamiltonian in SCF calculations and adds appropriate gradients for the EMBPOT potential.
The EMBPOT perturbation works by reading x, y, z, w, and v coordinates from a file (
EMBPOT) during the call toHF::form_H(). The core Hamiltonian of theHFobject is then modified to include a contribution that is calculated by performing numerical integration of the basis over the EMBPOT coordinates, weights, and potentials. This functionality assumed that the values of phi calculated in theBasisSet::compute_phi()routine are always in a cartesian basis and not in a spherical basis, which was true up until the fix in #2210, which was included in the 1.4 release. Accordingly, an unnecessary AO to SO transformation is applied in theHF::form_H()routine every time, which results in inaccurate EMBPOT matrices being added to the core Hamiltonian. In order to correct this in the current version of Psi4, I have removed the AO to SO transformation and callBasisSet::compute_phi()on an appropriately sized vector. I have also added numerical gradients over the EMBPOT potential using the gau2grid library.User API & Changelog headlines
EMBPOTfile. The first line of theEMBPOTfile must have the number of points inside of the file. The user must also includeset perturb_h trueandset perturb_with embpotin the Psi4 input. The potential will then be evaluated and included in the core Hamiltonian construction, and energy and gradient calculations.Dev notes & details
HF::form_H()by removing AO to SO transformation and supplying an appropriately sized vector to theBasisSet::compute_phi()call.MintsHelper::embpot_grad()to calculate the component of the gradient from the EMBPOT perturbation.Checklist
embpot1to compare energies and gradients computed by including embedded point charges analytically, through theexternal_potentialskeyword, and numerically, through the EMBPOT functionality.Status