[WIP] Additional exx kernels: Voxel averaged and Wigner-Seitz truncation#1282
[WIP] Additional exx kernels: Voxel averaged and Wigner-Seitz truncation#1282toschaefer wants to merge 5 commits into
Conversation
|
Nice! Particularly interested in the Wigner-Seitz truncation - is there any reason to do anything else? It seems to me this is the superior approach? Do we really want to support the voxel averaged? Also maybe some opportunities to share code with ewald.jl, but I haven't looked too closely. It'd be nice to have a generalized API for all these functions that return centered functions (we have a couple of those: densities guesses, local pseudos, projectors, coulomb kernels, etc.). I think long-term we want the ability to get their real-space and partial Fourier transforms (wrt 1 or 2 dimensions), so we can do eg 2D materials. But maybe it's not worth unifying since they have quite different requirements (numerical psp vs analytic but singular coulomb really are quite different computationally) CC @denizunel |
I think so, but I'd keep that for later, same as the generalized API. I'm currently looking at this on the side to streamline the code a bit, will hopefully push a review in the next week. |
For EXX of gapped systems Wigner-Seitz truncation is probably the best approach to reach the thermodynamic llimit.
We need it for surface science (strongly anisotropic cells) with coupled cluster theory. Since our plan is to make DFTK a standard workflow, voxel averaged would be essential. |
|
rebased to make merge easier |
|
Is this still WIP or ready for review? |
|
from my end, ready for review |
| - [`SphericallyTruncatedCoulomb`](@ref): θ(R-r)/r | ||
| - [`WignerSeitzTruncatedCoulomb`](@ref): χ(r)/r where χ(r)=1 inside Wigner-Seitz cell, otherwise 0. | ||
|
|
||
| If an interaction model features a singularity, that requires some special treatment, |
There was a problem hiding this comment.
no comma after singularity (germanism)
There was a problem hiding this comment.
features a singularity -> is long-range
| # where Γ is BZ volume) is used. | ||
| # (where Γ is BZ volume) is used. | ||
| # _compute_kernel_fourier(::InteractionKernel, basis, qpt, q) | ||
| # The single q-point version of compute_kernel_fourier |
There was a problem hiding this comment.
there's compute_ and eval_, should all these be merged?
There was a problem hiding this comment.
(we really need to have a unified API for localized functions that can be evaluated in real or reciprocal space)
There was a problem hiding this comment.
also should we have a eval_kernel_real? This is often useful for debug purposes (can be left as TODO if you don't feel like doing it)
There was a problem hiding this comment.
compute_ sets up and returns the full vector with all the complexity of how the sampling points (including the singularity) are defined for each G+q while eval_ is more the mathematical evaluation of the kernel at a given point G+q.
I added a TODO for eval_kernel_real.
|
|
||
| See also: [`compute_kernel_fourier`](@ref) | ||
| """ | ||
| abstract type InteractionKernel end |
There was a problem hiding this comment.
AbstractCoulombKernel? Interaction seems a bit generic (these are all Coulomb-like)
There was a problem hiding this comment.
After some back-and-forth we called it InteractionKernel in #1223 (comment)
| If an interaction model features a singularity, that requires some special treatment, | ||
| the following are available: | ||
| - [`ProbeCharge`](@ref): Gygi-Baldereschi probe charge method | ||
| - [`ReplaceSingularity`](@ref): Set G+q=0 component to given value (default is zero) |
There was a problem hiding this comment.
I think we use relatively consistently (or at least we wanted to) p for the Fourier-space variable in \R^3 (so usually things like p=G+k, and q=k-k'). Why mention G at all in this file? Can't you just use p?
There was a problem hiding this comment.
Principally, I am happy to follow your stylistic preferences. I don't think we've mentioned this yet during the EXX development, and if I shall change it, I would prefer to postpone it for now, since I'm already working on the next PR (which also includes some changes in src/coulomb.jl).
| Base.Broadcast.broadcastable(k::InteractionKernel) = Ref(k) | ||
|
|
||
| # Each InteractionKernel should support the following functions: | ||
| # eval_kernel_fourier(::InteractionKernel, Gsq) |
There was a problem hiding this comment.
give the equation defining this. Gsq -> p? Or ps?)
There was a problem hiding this comment.
Gsq refers to G squared so psq? But I am not sure if I understand correctly...
| d_min = min(d_min, d) | ||
| end | ||
| if d_min > sqrt(eps(T)) | ||
| V_lr_real[idx] = erf(ω * d_min) / d_min |
There was a problem hiding this comment.
as above, define a erf(x)/x function?
There was a problem hiding this comment.
Hm I think we should have a general way of doing f(x)/x stably and AD-friendly, this is coming up pretty frequently (also with @Technici4n in the spherical harmonics stuff)
There was a problem hiding this comment.
Ah but of course it's my old friend the divided difference (high order in the case of f(x)/x^l). We have a general solution in https://github.com/xuequan818/MatrixFuns.jl but it might be a bit overkill...?
There was a problem hiding this comment.
(the reason I want to get this right is otherwise it will come up to bite us in the ass with AD)
There was a problem hiding this comment.
I want to do things like
function expm1_over_x(x)
if abs(x) < 1000floatmin(Float64)
x = 1000floatmin(Float64)
end
expm1(x) / x
end
but of course this screws up AD at 0. We can have a cutoff point and switch to a finite order taylor expansion. This is ugly and getting good precision of higher derivatives is annoying, but I'm not sure what else to do.
| V_lr_real = zeros(Complex{T}, basis.fft_size...) | ||
| for idx in CartesianIndices(V_lr_real) | ||
| r_frac = r_vectors[idx] | ||
| r_centered = r_frac .- round.(r_frac) # MIC |
There was a problem hiding this comment.
write it out explicitly (I had to think to get what it was referring to)
| r_centered = r_frac .- round.(r_frac) # MIC | ||
| r_cart = model.lattice * r_centered | ||
| d_min = norm(r_cart) | ||
| for dx in -nx:nx, dy in -ny:ny, dz in -nz:nz # Check neighbors for non-orthorhombic cells |
There was a problem hiding this comment.
I have to say I don't understand what's going on here, why isn't this just erf(r)/r for r in r_vectors?, maybe factor out the geometry stuff to another function?
| Gnorm2 = sum(abs2, G_cart) | ||
| found_singularity = (iG==1 && iszero(q)) | ||
| Rcut = cbrt(basis.model.unit_cell_volume*3/4/π) | ||
| if !found_singularity |
There was a problem hiding this comment.
why don't you just write the condition here? Also why not iszero(G_cart)?
There was a problem hiding this comment.
(also same comment as above, hopefully we can avoid this special casing)
| end | ||
| kernel_fourier | ||
| end | ||
|
|
There was a problem hiding this comment.
Running out of time today, note to self to resume the review from here.
Two additional useful strategies for the Coulomb kernel for electron repulsion integrals (e.g. for exx).
Wigner-Seitz truncation
R. Sundararaman, T. A. Arias. Phys. Rev. B 87, 165122 (2013)
Voxel averaged (mean potential)
T. Schaefer et al, J. Chem. Phys. 160, 051101 (2024)
This requires Gauss-Legendre quadrature points, hence I added the package FastGaussQuadrature to the dependencies.