search

JuliaMolSim / DFTK.jl

Density-functional toolkit
https://docs.dftk.org
MIT License
428 stars 89 forks source link

Use the Simpson rule for integration of local part of PSPs #982

Closed abussy closed 3 months ago

abussy commented 3 months ago

This PR switches the integration of the local part of the pseudopotentials on radial grids from the trapezoidal to the Simpson's method.

This change induces a shift in the total energy, due to the PspCorrection and AtomicLocal terms. This is a rigid shift: the electronic density, energy gradients, and the physics are not modified. This energy shift is in the order of 10^-5 Ha, and the exact number depends on the system and the choice of pseudopotential.

As far as my numerical analysis understanding goes, the Simpson's method should be more accurate. My main motivation for this PR however, is that it makes DFTK total energies more comparable to other codes (tested against QE and SIRIUS).

Note: despite that shift in total energies, all the tests pass on my local machine. Are the tolerance threshold a bit too loose, maybe?

antoine-levitt commented 3 months ago

Makes sense to me, why did we have this again?

antoine-levitt commented 3 months ago

This is a rigid shift: the electronic density, energy gradients, and the physics are not modified

Why do you say that? It should also change these, no?

abussy commented 3 months ago

This is based on my observations: the only non-negligible changes in the energy breakdown were on the PspCorrection and AtomicLocal terms. The Xc and Hartree terms, which only depend on the density, have changes in the order of 10^-6 Ha or less.

I think the density might in principle be affected, but in a very minor way.