-
Notifications
You must be signed in to change notification settings - Fork 87
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
Polish dielectric example and improve chi0 computation #286
Conversation
@@ -180,7 +186,7 @@ is false, only compute excitations inside the provided orbitals. | |||
# ∑_{n,m != n} (fn-fm)/(εn-εm) ρnm <ρmn|δV> | |||
ρnm = conj(ψnk_real) .* ψmk_real | |||
weight = dVol * dot(ρnm, δV) | |||
δρk .+= real(ratio .* weight .* ρnm) | |||
δρk .+= (n == m ? 1 : 2) * real(ratio .* weight .* ρnm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment either here or in the loop above? for m = n:nbands
is very very easy to confuse with for m=1:nbands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also fuse the multiplication
@@ -19,7 +19,7 @@ script: | # Install dependencies and test with coverage | |||
|
|||
# Some optional python packages via conda | |||
using Conda | |||
for pkg in ["ase", "matplotlib =3.2.2"] | |||
for pkg in ["nomkl", "ase", "matplotlib =3.2.2"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment for why this is necessary?
dρ = apply_χ0(scfres.ham, scfres.ψ, scfres.εF, scfres.eigenvalues, from_real(basis, dv)) | ||
Kdρ = apply_kernel(basis, dρ; ρ=scfres.ρ) | ||
vec(dv - Kdρ.real) | ||
# Apply ε = 1 - χ0 (vc + fxc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eps is the dielectric operator, and applies to potentials conventionally. Call that epsT or something?
end | ||
end | ||
end | ||
howmany = 5 | ||
arnoldi(eps_fun, vec(randn(size(scfres.ρ.real))), howmany) | ||
arnoldi(eps_fun, vec(randn(size(scfres.ρ.real))); howmany=5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should replace that by krylovkit's native implementation anyway now that they have the "greedy" option. @gkemlin is doing that, we'll just merge later
@@ -130,7 +133,7 @@ Solve the Kohn-Sham equations with a SCF algorithm, starting at ρ. | |||
# Callback is run one last time with final state to allow callback to clean up | |||
info = (ham=ham, basis=basis, energies=energies, converged=fpres.converged, | |||
ρ=ρout, eigenvalues=eigenvalues, occupation=occupation, εF=εF, | |||
n_iter=n_iter, ψ=ψ, stage=:finalize) | |||
n_iter=n_iter, ψ=ψ, diagonalization=info.diagonalization, stage=:finalize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diagonalization_info? We probably need a more consistent naming for nested named tuples. Eg args for input and info for output, so this could be diag_info and the input parameters diag_args
No description provided.