JuliaMolSim / DFTK.jl

Density-functional toolkit
https://docs.dftk.org
MIT License
444 stars 89 forks source link

Calculator State #961

Closed CedricTravelletti closed 3 months ago

CedricTravelletti commented 8 months ago

This introduces state updating in the calculator.

See this PR in GeometryOptimization.jl for a detailed discussion of why this is needed.

mfherbst commented 8 months ago

Lets wait for https://github.com/JuliaMolSim/AtomsCalculators.jl/pull/11 and see where this settles.

BTW: It's a bit weird your PRs always show these millions of commits if you only add a few things. Any idea what that could be ?

CedricTravelletti commented 8 months ago

Yes, I know why there are so many commits: What is done in this PR (and in the other ones I submitted) is part of a cross-package effort.

What I mean is that the PR in DFTK.jl is just a side effect of what we want to do in GeometryOptimization.jl, but changes will also then ripple on to AtomsCalculators.jl (and in the end we want to use everything in InverseDesign.jl).

So things evolve in parallel and create a back and forth commit cascade before the final PR is settled.

mfherbst commented 3 months ago

Just pushed an update to get this compatible to AtomsCalculator 0.2. There is a key issue remaining: We are not able to benefit from reusing state in calls like energy_forces or calculate((Energy(), Forces()). This is due to some missing features in AtomsCalculators, which I don't have time to fix right now.

In short:

I would still suggest we merge as is, just to have the update to 0.2 done and then proceed to fix the issues in AtomsCalculators.

See https://github.com/JuliaMolSim/AtomsCalculators.jl/issues/27