Skip to content
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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Jul 24, 2023

Proposed changes

  1. Make SPOSet a class template
  2. port FakeSPO based on SPOSetT with minimal change
  3. always use extern template to avoid redundant compilation.

What type(s) of changes does this code introduce?

  • New feature

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

laptop

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'

@jtkrogel
Copy link
Contributor

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.

@ye-luo
Copy link
Contributor Author

ye-luo commented Jul 25, 2023

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.

@prckent
Copy link
Contributor

prckent commented Jul 25, 2023

@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.

@jtkrogel
Copy link
Contributor

jtkrogel commented Jul 25, 2023

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).

See e.g.: https://github.com/QMCPACK/qmcpack/blob/develop/tests/solids/diamondC_1x1x1_pp/qmc_1rdm_noJ_short.in.xml

src/QMCWaveFunctions/SPOSet.h Outdated Show resolved Hide resolved
src/QMCWaveFunctions/SPOSet.h Outdated Show resolved Hide resolved
@ye-luo ye-luo changed the title [WIP] Make SPOSet a template. Make SPOSet a template. Nov 9, 2023
@ye-luo ye-luo changed the title Make SPOSet a template. Make SPOSet a class template Nov 9, 2023
@ye-luo
Copy link
Contributor Author

ye-luo commented Nov 9, 2023

Test this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants