Add customizable FREEZE_CORE policy option #2667
Conversation
This patch adds an option to FREEZE_CORE called "policy", which enables frozen core settings to be looked up from a list specified in the global variable FREEZE_CORE_POLICY. This is more flexible than NUM_FROZEN_DOCC for situations like SAPT where multiple molecules are run in the same command and may require different individual numbers of frozen cores, with settings more customizable than TRUE/FALSE/1/0/-1/-2
|
This is a little confusing to me. It looks like basisset.cc line 240 accounts for Z = 0 but then the example array in the documentation at read_options.cc line 126 seems to have the Z = 0 in it. Or the example array could be mistyped (H-Be is just four atoms while five zeros are given). Then the example input file has the Sb atom (Z = 51) but I'm not saying whether or not including Z=0 in the array is correct but documentation should be very clear. |
Good catch! It was in fact a typo on my part, I've fixed it + added a little more clarification in the documentation for |
| @@ -84,6 +84,15 @@ E, wfn = energy("mp2/def2-svp", return_wfn=True, molecule=sbh3) | |||
| compare_values(wfn.nfrzc(), 4, 1, "Number of frozen orbitals with freeze_core = 1") | |||
| compare_values(sum(wfn.doccpi()), 13, 1, "Number of occupied orbitals with freeze_core = 1") | |||
|
|
|||
| set freeze_core policy | |||
| core_policy = [0 for _ in range(51)] | |||
| core_policy[50] = 5 | |||
There was a problem hiding this comment.
I would comment here, too, acknowledging that Sb is Z=51 but we need to set the Z-1 element in the array.
|
Just one small addition to make and then it looks good to me. |
|
Updated! Tbh, for large elements the use starts to get real clunky, but I don't understand the codebase well enough to instead provide a dict vs an array. Someone savvier than me should probably eventually update that (so that a user doesn't need to set 0's for a bunch of elements they Just Don't Care About), but I didn't see a mapping type available in the relevant context (just an int vector) so I went with what was there. |
|
Ah, yeah, for large elements it will get clunky. We do have |
|
I've authorized the test suite to run. This is our way of confirming that nothing is obviously broken. If everything passes (as I expect it should), there's nothing on your end yet to do. If it fails, give it a quick look and flag Lori or I if you need assistance in identifying the issue. |
|
@JonathonMisiewicz looks like most things worked except one of the linux builds failed for an issue that I don't think is related to my patch: Could NOT find Python (missing: Python_NumPy_INCLUDE_DIRS Interpreter
NumPy) (found suitable version "3.8.10", minimum required is "3.8")Not sure how to proceed here. |
|
I've restarted it, which has worked before. If it's the same problem as I've seen on Windows, it has to do with the latest NumPy. Haven't investigated why its sporadic on Linux yet though. |
There was a problem hiding this comment.
Thanks for the contribution! My first thought when I heard arrays for frozen-core was per-atom in mol, so that one could specialize like basis set. But that's something to consider for a future basis input rework. This solves the immediate problem and accords with the described workflow, so lgtm.
Description
This patch adds an option to FREEZE_CORE called "policy", which enables frozen core settings to be looked up from a list specified in the global variable FREEZE_CORE_POLICY. This is more flexible than NUM_FROZEN_DOCC for situations like SAPT where multiple molecules are run in the same command and may require different individual numbers of frozen cores, with settings more customizable than TRUE/FALSE/1/0/-1/-2
This patch addresses some (but not all) issues raised in #2631 by allowing for more flexible policies to be set appropriate to multi-part calculations.
Todos
Checklist
tests/dfmp2-ecp/input.datctest -L quickruns successfully, which includes the above listed testcteststill in-flight but given the scope of this patch I don't expect any issuesStatus