Skip to content

Conversation

@johnppederson
Copy link
Contributor

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 to HF::form_H(). The core Hamiltonian of the HF object 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 the BasisSet::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 the HF::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 call BasisSet::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

  • The user may supply an arbitrary potential evaluated on a numerical quadrature grid to an SCF calculation by saving the x, y, z, w, and v values to a human-readable EMBPOT file. The first line of the EMBPOT file must have the number of points inside of the file. The user must also include set perturb_h true and set perturb_with embpot in the Psi4 input. The potential will then be evaluated and included in the core Hamiltonian construction, and energy and gradient calculations.

Dev notes & details

  • Fixed the EMBPOT functionality in HF::form_H() by removing AO to SO transformation and supplying an appropriately sized vector to the BasisSet::compute_phi() call.
  • Added function MintsHelper::embpot_grad() to calculate the component of the gradient from the EMBPOT perturbation.

Checklist

  • Added test embpot1 to compare energies and gradients computed by including embedded point charges analytically, through the external_potentials keyword, and numerically, through the EMBPOT functionality.
  • All or relevant fraction of full tests run

Status

  • Ready for review
  • Ready for merge

@loriab loriab added this to the Psi4 1.10 milestone Apr 29, 2025
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 bugfix, the new deriv functionality, and the tidy validating test case.

@loriab loriab moved this to LAB Done in LAB's v1.10 Release May 21, 2025

FILE* input = fopen("EMBPOT", "r");
int npoints;
int statusvalue = fscanf(input, "%d", &npoints);
Copy link
Member

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.

Copy link
Contributor Author

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.

@loriab
Copy link
Member

loriab commented May 21, 2025

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.

Copy link
Member

@jturney jturney 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 changes!

@loriab loriab enabled auto-merge May 22, 2025 16:52
@loriab loriab added this pull request to the merge queue May 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 22, 2025
@loriab loriab added this pull request to the merge queue May 22, 2025
@loriab
Copy link
Member

loriab commented May 22, 2025

merge_queue failed on test cc1 on Windows UnicodeDecodeError: 'utf-8' codec can't decode byte 0x92 in position 1004: invalid start byte. haven't seen that one before, so hopefully a fluke.

Merged via the queue into psi4:master with commit 7e18dcd May 22, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from LAB Done to Done in LAB's v1.10 Release May 22, 2025
@loriab loriab mentioned this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants