Skip to content

Conversation

@wavefunction91
Copy link
Member

Remove Molecule input from XC property types

@wavefunction91 wavefunction91 marked this pull request as ready for review August 29, 2022 23:50
Copy link
Member

@ryanmrichard ryanmrichard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is how we did it before, but this property type still sits weird with me. In short, the XC operator is a one-electron operator and AO spaces are one-electron basis sets. This means there's really only enough input information to build the tensor representation of a one-electron operator and not enough information to compute a many-electron energy (for starters you don't have the number of electrons).

This doesn't need to be resolved now, but I think it's worth thinking about to ensure logical consistency across the framework. My guess is this should probably be a specialization of BraKet like our other many electron quantities (Assuming I understand DFT correctly this in turn assumes we're specifically doing Kohn-Sham DFT. I'm not sure what it'd look like for non-KS, maybe the one-electron density and the xc operator in the basis of the one-electron density?).

@ryanmrichard ryanmrichard merged commit c7cfcd4 into master Aug 30, 2022
@ryanmrichard ryanmrichard deleted the xc branch August 30, 2022 14:17
@wavefunction91
Copy link
Member Author

TL;DR - we can unify them, just make an issue so we can keep track of what we "want"

I know this is how we did it before

I take it you mean viz the ExchangeCorrelationEnergy PT, correct? Because this PR does make the ExchangeCorrelationPotential a specialization of TwoCenterAOTensorRepresentation like everything else.

This means there's really only enough input information to build the tensor representation of a one-electron operator and not enough information to compute a many-electron energy

This isn't really true - we definitely have enough information to compute the energy associated with this operator given a canonical orbital space, it's just not computed the way that linear/quadratic functionals of the energy (e.g. everything else we work with) are evaluated - it's a nonlinear functional that must be integrated rather than it simply being the trace of the AO representation of an operator with an associated density matrix.

I'm all for unifying the implementation with BraKet - we would just need an assurance down stream that any generic module for computing arbitrary BraKet expectation values wouldn't try to handle EXC in the same way as the other operators. How we make that a guarantee is a bit opaque to me, but if we want to work under he assumption that we're realistically the only ones who are going to modify this code in the near future (and add appropriate dox), I'm all for combining them if it makes things cleaner.

If we think of BraKet as the energy expectation value associated with a particular one-body operator, which it is in the KS framework, then I think this is logically consistent.

I'm not sure what it'd look like for non-KS, maybe the one-electron density and the xc operator in the basis of the one-electron density?

"Basis of the one-electron density" sounds a bit awkward - I think this just stems from the fact that DFT in general is derived as an energy ansatz, i.e. the total electronic energy is defined to take the form

E[rho] = T_s[rho] + V[rho] + E_XC[rho]

The whole idea of KS is just to construct rho from a basis of single particle orbitals. In that sense, the density is the more fundamental variable, and its relationship to a particular pure (single determinant) state is imposed by the KS ansatz. You can also do this with MC states - just see the stuff that Gagliardi does

@ryanmrichard
Copy link
Member

Sorry, I now see that I completely misread the diff. I originally thought the ExchangeCorrelationEnergy PT was replacing the ExchangeCorrelationPotential PT, but that's not at all what happened. Making ExchangeCorrelationPotential a specialization of TwoCenterAOTensorRepresentation is the way to go.

As for making EXC a specialization of BraKet, we don't have to do this now (I'll open an issue to revisit this), but I do think it would simplify the code if we do so (in particular I think we get rid of the edge case in the energy evaluation). The name BraKet is perhaps not the greatest, but the BraKet property type covers matrix elements between two (possibly different) many-electron wavefunctions; so it's not just for expectation values. In the context of general DFT, there's no wavefunction so BraKet doesn't make sense, but at least for KS DFT there is a wavefunction so BraKet seems viable. BraKet is templated on the bra and ket wavefunction types, so if it doesn't make sense for BraKet to be defined for anything other than an expectation value we can restrict bra == ket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants