Skip to content

add τ in energy_hamiltonian required for some TermXc#1100

Merged
mfherbst merged 5 commits into
JuliaMolSim:masterfrom
aouinaayoub:master
May 24, 2025
Merged

add τ in energy_hamiltonian required for some TermXc#1100
mfherbst merged 5 commits into
JuliaMolSim:masterfrom
aouinaayoub:master

Conversation

@aouinaayoub

Copy link
Copy Markdown
Contributor

if we use some meta-GGA functional, save and load the scfres with skip_hamiltonian=false (default), it causes energy_hamiltonian to error due to the missing kinetic energy density τ key, despite τ being available in the saved scfres

@mfherbst

Copy link
Copy Markdown
Member

Thanks for observing this, very much agree something like this should be around.

The problem is this way you assume \tau to be always part of the jld, which may not be the case. I'd fence this off with some extra checks.

I think we have also a function to check whether \tau is required for the hamiltonian (see SCF).

@aouinaayoub

Copy link
Copy Markdown
Contributor Author

Looking at how quantities are collected in scfres (https://github.com/JuliaMolSim/DFTK.jl/blob/master/src/scf/self_consistent_field.jl#L227), it seems τ will always be part of the jld, it's set to nothing when it's not needed.

@mfherbst

Copy link
Copy Markdown
Member

I see, but that's actually not intended behaviour. The save_scfres has a flag save_ρ which controls whether ρ is saved (to be able to save disk space in case only high-level results such as the energy are needed). I'd argue the same flag should control whether τ is saved, too. So don't rely one a scfres[:τ] to be present.

I'd do something like

https://github.com/JuliaMolSim/DFTK.jl/blob/master/ext/DFTKJLD2Ext.jl#L105

also for τ and modify appropriately the check

https://github.com/JuliaMolSim/DFTK.jl/blob/master/ext/DFTKJLD2Ext.jl#L151

such that if τ is nothing, but the ham needs τ the ham is not built.

Probably we should also modify the behaviour of saving τ when save_ρ=false and document it appropriately.

@aouinaayoub

Copy link
Copy Markdown
Contributor Author

I set τ to be retrieved from the dictionary or set to nothing, then check if a term in the basis needs τ before calling energy_hamiltonian.

@mfherbst mfherbst enabled auto-merge (squash) May 24, 2025 12:20
@mfherbst

Copy link
Copy Markdown
Member

There were some unrelated issues in the CI I had to fix plus I added the extra features I briefly mentioned on saving the scfres.

@mfherbst mfherbst disabled auto-merge May 24, 2025 17:25
@mfherbst mfherbst merged commit 1e889ec into JuliaMolSim:master May 24, 2025
niklasschmitz pushed a commit that referenced this pull request Nov 17, 2025
Co-authored-by: Michael F. Herbst <info@michael-herbst.com>
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.

2 participants