-
Notifications
You must be signed in to change notification settings - Fork 21
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
Psi4 and PCMSolver v1.3 and UFF radii #201
Comments
Hei Lori! Thanks for pursuing the conda-forge packaging. I'd be happy to be included in the maintainers, though, as you wrote, the library is seeing only minimal maintenance these days. The differences you report do appear to be consistent with a change in radii, so it's OK to update the reference values in Psi4. Can you refresh my memory as to why the UFF radii should not be multiplied by 1.1? As for which stable line to package: I'm in the (slow) process of finish up a new patch release of 1.3.x in #200 If you start the package to follow line 1.2.x, I think when 1.3.1 is out the bots will update it. So, I think it's fine to get 1.2.3 on conda-forge at first. Once that is in I'll mint 1.3.1 and Psi4 can work with the 1.3.x line. |
Great, thanks!
I really hadn't thought beyond "aha, this is the root of the discrepancy". Are you saying that the energies in psi4 shouldn't be changing that much and that instead there should be a compensatory 1.1 scaling somewhere in psi? Also, I noticed that 1.3.x introduced a new interface where one sets an empty object, then sets options by API. Psi is still using the old
Good news on the 1.3.1 release. Note that I've got some cmake updates at release/1.2...loriab:pcmsolver:v123_plus_ming (includes #193 ) that it'd be nice to get into the new release once they settle down. |
Hi @robertodr, don't worry, this isn't a new error. This is an update on that old trouble of PCMSolver v1.3 failing on several of the Psi4 tests; you and I both tried to solve it a couple years ago to no avail. Psi4 since has been using v1.2.1 plus 3 parsing commits, informally dubbed v1.2.1.1.
Anyways, I've been poking at these problems preliminary to putting pcmsolver on conda-forge. Is that ok in general and do you want to be a maintainer on the recipe? I got Psi4 using v1.2.3 with very small tweaks (~1.e-8) to test cases after you introduced the PEDRA pruning. (And I added the citation printing.) So continuing with the v1.2.x branch is one option. Then today, I gave v1.3.0 a try again backing out the 1.0 -> 1.1 UFF radii conversion https://github.com/PCMSolver/pcmsolver/blob/master/src/utils/Atom.cpp#L182 and happily, that heals all the Psi4+PCMSolver tests. Note that the radii change was influencing energies in the third decimal place (see error log below). Does that seem right order-of-magnitude? So updating Psi4 ref values and switching to v1.3.x branch is a viable option.
I quite understand that PCMSolver is under minimal development, and I'm not pressuring otherwise. I wanted to see if you had any advice for which branch to pursue for Psi4 and for c-f packaging. Normally, I'd say v1.3.x, of course, but I don't know if it got as much exercise downstream as v1.2.x. I'd be glad of any advice.
The text was updated successfully, but these errors were encountered: