DFTK.jl
DFTK.jl copied to clipboard
JuliaMolSim
Struct for psi and its friends
Just to keep track of that. It'd be nice to have a struct that contains ham, psi, rho, occupations, fermi level, orbital energies, etc. and that is strongly immutable (in the sense that its bit contents don't change after creation). I think the major question is: does psi belong with the hamiltonian it creates or with the density that created it? The first one seems more natural, but is trickier to express mixing algorithms in (because a hamiltonian is created by a density, without reference to a psi). This is specific to DFT, as opposed to eg hybrid functionals where you need a psi to make the hamiltonian also.
Possibly the fact that I'm having trouble figuring this out precisely means we should keep these concepts separate, and just have a struct for scfres like we have right now.
Maybe the way to go is actually to have a state datastructure collecting the results we have obtained from a DFT run in one datastructure.
We might want to revisit this.
psi, occ, rho, rho_diff for spin, orbital energies, fermi level, possibly a few other fields. I think the only sensible way to do it is to have all fields nullable, which isn't great but I don't see any other way. Usage: pass to energy_hamiltonian, to the terms, to the callbacks, return from the SCFs. We can have a constructor that takes psi and energies and computes the rest, which would simplify the code in the SCF. Let's discuss that sometime.
So the only difference to the named tuple would be the constructor, do I get that right? In that case I would have a function to do that in a structured way. I really do not see why you do need a proper struct for that. In my experience with structured objects for this sort of result state, it often happens that they get very quickly very clunky (because it is so convenient to stuff everything into it). So I really would be careful with it. But yes, let's talk about it.
Actually now that I see a little the code of the new structure I see what you mean. It is really super annoying to pass basis and ρ around for everything.
Right. The problem is that there are many uses we can do of such a structure:
- Postprocessing
- Compute an energy
- Compute a Hamiltonian
Each of these use different parts (eg, at least without hybrids, we can compute a Hamiltonian just with just the density). Consistency is also tricky: if the struct contains psi, occupation and rho, does it mean that rho is constructed from psi and occupation? Or that psi and occupation are constructed from the hamiltonian associated to rho? That's why I was suggesting having all fields as nullable, and not imposing too strict a contract on them. Could go either way as to whether we want a struct or a named tuple. Structs have the advantage of readability: when you see state::ElectronicState, you just have to look at ElectronicState to see what's in there.
Also in favor of state: gives a unified place for the documentation of these things, eg the storage format of psi