-
Notifications
You must be signed in to change notification settings - Fork 140
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
Make SPOSet a class template #4685
base: develop
Are you sure you want to change the base?
Conversation
Is there any possibility for odd interactions with classes like CompositeSPOSet? That class merges other SPOSets into a single one. Since with this PR each SPOSet might have a distinct value type, I was wondering if that might produce odd corner cases and/or break that class for some value type combinations. |
The type/precision restriction from the base class only applies to the API. The derived class has all the freedom to use any type internally, for example single precision splines orbitals in the full precision build. Making SPOSet template doesn't change the fact that this restriction applies to CompositeSPOSet when it talks to its owned SPOSets but the latter have all the freedom to make their internal precision individually. |
@jtkrogel Wondering if your https://doi.org/10.1063/1.4994817 went through the composite SPOset path? I don't think we have an example of this use case and that paper was a good justification for the generality of the present code. Another example would be supporting CP2K's Gaussian+plane wave basis, but I don't think we have even the beginnings of a converter. |
Yes, that paper used the composite sposet path. The 1RDM code also uses it currently to support memory savings (make one sposet for occupied and one for virtual; determinantset uses only the occupied sposet while density matrix merges the two for use as a basis). |
ed30455
to
198909f
Compare
Test this please |
7840f96
to
98f57f4
Compare
Proposed changes
What type(s) of changes does this code introduce?
Does this introduce a breaking change?
What systems has this change been tested on?
laptop
Checklist