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

Include tabulated Hartree-Fock wave functions for atoms #71

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

susilehtola
Copy link
Collaborator

@susilehtola susilehtola commented Aug 11, 2023

Description

Atomic densities are necessary for certain algorithms.

One possibility, used in some codes is to use Slater atomic densities, which are empirical exponential fits of the observed electron density. However, I have not found up-to-date data for such an implementation, but it can be added later.

A perhaps better alternative is to use Slater-type orbital wave functions from the literature, which should be relatively accurate. This PR is meant to include these tabulations in a reasonable fashion.

Copy link
Owner

@wavefunction91 wavefunction91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How long does this take to compile? Also, were the heavy-element calculations performed with point- or finite-nuclear models?

~SlaterTypeAtomicShell() {};

/// Evaluates the basis functions
std::vector<double> evaluate_basis_functions(double r) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning std::vector is an anti-pattern, we should always expose the capability to populate preallocated memory. I'd probably opt to have something like gau2grid for Slaters, but of course with a lot less logic / having the ability to trigger fast code paths for spherically averaged functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've exposed the capabilities at least partly. Have a look

@wavefunction91
Copy link
Owner

Required for #60

@susilehtola
Copy link
Collaborator Author

The code is now in .cpp files. Should these be headers as well?

Copy link
Owner

@wavefunction91 wavefunction91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re source vs header, I could go either way. I think were approaching enough performance-critical functionality that we may have to abandon header-only, or at least we should expose the option to precompile

@wavefunction91
Copy link
Owner

@susilehtola Refresh my memory - what's left here? Decisions on source organization? Functionality? My only concern about checking in what's here is that, by itself, it's not terribly interesting, but when its combined with e.g. a weight partitioning scheme, then its in scope. Might need to be merged with #70 or at least held off until that's checked in and added when we add an atomic partitioning that needs it.

@susilehtola
Copy link
Collaborator Author

In addition to use in the Delley partitioning, #60, atomic densities are also needed for the adaptive Molpro grid, wavefunction91/GauXC#51.

Atomic densities could also be useful for the adaptive determination of atomic radial grids, since the quadrature error is atom and functional dependent, and the quadrature errors are similar for HF and SCF wave functions; one would then only have to define an energy threshold. It could also be useful to port the code I currently have in https://github.com/JFurness1/AtomicOrbitals to C++ because running the total energy convergence plots we introduced in J. Chem. Phys. 157, 174114 (2022) takes a while in Python... These features would just require writing a Libxc interface, which could be compiled as an optional binary. What do you think?

@wavefunction91
Copy link
Owner

@susilehtola Do you mean to move this kind of database / evaluator functionality to the AtomicOrbitals repo? Or to add that C++ implementation here? I think that the former could be much cleaner - if that can be coupled with a proper CMake harness, it could just be pulled in to IntegratorXX as a dependency.

@susilehtola
Copy link
Collaborator Author

AtomicOrbitals is a pure Python package. I was thinking of including the evaluator here as an optional binary, but that code can of course live somewhere else like a new project. However, the atomic wave functions should be in IntegratorXX since we need them for several planned features!

@susilehtola
Copy link
Collaborator Author

OK, now the code should be working. I get the same energies with the C++ implementation as with AtomicOrbitals.

Now the question is how do you want to interface this into the atomic partitioning generation. Also, I guess some tests should be added.

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.

2 participants