Phonons docs #953
Phonons docs #953
Conversation
|
@mfherbst not up for review yet, still ongoing with Etienne |
src/PlaneWaveBasis.jl
Outdated
| # By default kpoints_phonon has only kpoints | ||
| kpoints_phonon = IdDict{Tuple{Vec3{T}, Int}, | ||
| NamedTuple{(:kpt, :equivalent_kpt), Tuple{Kpoint, Kpoint}}}() | ||
| for kpt in kpoints | ||
| key = _kpoint_phonon_key(kpt.coordinate, kpt.spin) | ||
| kpoints_phonon[key] = (; kpt, equivalent_kpt=kpt) | ||
| end |
There was a problem hiding this comment.
Not sure about how to do this properly… Feels bloated.
There was a problem hiding this comment.
And fails for ForwardDiff because of round. 93c0777
src/transfer.jl
Outdated
| Note that `kpt_out` does not have to belong to `basis_out` as long as it is equivalent to | ||
| some other point in it, i.e, `kpt_in = kpt_out + ΔG`. | ||
| Beware: `ψk_out` can lose information if the shift `ΔG` is large or if the `G_vectors` | ||
| differ between `k`-points. |
There was a problem hiding this comment.
@antoine-levitt if that's what you had in mind.
|
Otherwise (pending the awful |
|
Didn't we say we could get rid of memoization by doing get_kpoint_from_equivalent(kcoord, equiv_kpoint) and just translating the G vectors from the equivalent kpoint? |
|
Yep, sorry I forgot. Will change that. |
|
I think I still need to recompute the mapping each time. So it would still be costly. Or did I miss something? |
|
We should be able to deduce it from the one of the old kpoint, no? I'm not super concerned about it being very efficient, worse case we just recompute everything... |
|
Done. Less awful than I thought it would be, but lots of conversions. |
src/transfer.jl
Outdated
| # Mapping is the same as if created from scratch, although it is not ordered. | ||
| mapping = map(CartesianIndices(basis.fft_size)[equivalent_kpt.mapping]) do G | ||
| linear[CartesianIndex(mod1.(Tuple(G + CartesianIndex(ΔG...)), basis.fft_size))] | ||
| end |
There was a problem hiding this comment.
That means we can not use a k-point of Kpoint with a k-point of get_kpoint. Is that ok? Or should I sort this list, so it should be equivalent?
There was a problem hiding this comment.
What do you mean we can't use it? Everything looks fine and consistent to me, we don't assume ordering anywhere? Maybe just add to Kpoint's docstring that it's not assumed to be ordered? (we do assume ordering of G_vectors(basis) however)
There was a problem hiding this comment.
In 04ec78c, if I do not replace Kpoint(…) by get_kpoint, the tests fails, the order of the FFT arrays become inconsistent.
I will add doc to Kpoint.
|
Good to merge for me when you want @epolack |
|
I'm good with it, thanks! |
|
Nice, looks good. |
No description provided.