Lattice Optimization

[CedricTravelletti]

This PR implements lattice optimization, following the ASE implementation.

[mfherbst]

Why is there DFTK-specific stuff in here, that should not be the case.

[mfherbst]

Ah I see you did this because there is no generic way to do this right now. This is a flaw in AtomsCalculators and something we should change.

My suggestion for what to do would be the following. In AtomsCalculators you introduce a calculator_state(calculator) function, which returns a state object and a method update_state(algorithm, state, original_system, new_system), which does some magic to update the state. We supply two fallbacks

struct DummyState end
calculator_state(::AbstractCalculator) = DummyState()
update_state(::Any, ::DummyState, ::Any, ::Any) = DummyState()

in AtomsCalculators. In this way there is zero additional implementation cost for people who don't want to by into this, but for DFT methods it helps.

In DFTK we then add the code you put here with

function update_state(algorithm, state::DFTKState, original_system, new_system)
    if bounding_box(original_system) != bounding_box(new_system)
        DFTK.DFTKState()
    else
        state
    end
end

and additionally

calculator_state(calc::DFTKCalculator) = calc.state

The rest I guess is clear: In here you just use calculator_state instead of accessing calc.state directly to make sure the dummy stuff works as well.

Note that I added additionally an algorithm argument, which you should expose to minimize_energy!. In this way the interpolation algorithm can be easily swapped.

[CedricTravelletti]

Done. Added PR to DFTK and AtomsCalculators implementing the changes.

[mfherbst]

We need a function to extract the state from the calcuator. Not all calculators may have state in which case the returned state would just be a dummy.

[mfherbst]

Also I think I would use a marker struct on the update_state, which simplifies later to provide different "update algorithms". We can discuss offline what I have in mind here.

[mfherbst]

Don't do this procedure thing. Think about the separation of "what" and "how". The procedure selects the "how", so it should be a type 😄.

On a practical level this means you should use a marker struct. By the way if you use marker structs, then you can also use these to automatically adapt your definition of f and g! in the above lines of code depending on the marker structs. This makes it super easy to later try other parametrisations or introduce more complicated claming techniques etc.

[mfherbst]

???

[CedricTravelletti]

Now removed.

[cortner]

what is the situation here? I was about to write an alternative geometry optimization wrapper porting the JuLIP implementation. But if this is ready or close to ready, then I will hold off.

[cortner]

sorry, I meant to just leave a single comment. Just let me know in the conversation if this is on track or you need help or I should do something separate to meet my current requirements.

[cortner]

I think you are messing with the types here, and also below in full_to_voigt.

julia> function full_to_voigt(A)
           [A[1, 1], A[2, 2], A[3, 3], 2. * A[2, 3], 2. * A[1, 3], 2. * A[1, 2]]
       end
full_to_voigt (generic function with 1 method)

julia> 

julia> A = rand(Float32, 3,3)
3×3 Matrix{Float32}:
 0.934422  0.399876  0.339769
 0.241855  0.856102  0.199015
 0.573214  0.554009  0.683092

julia> full_to_voigt(A)
6-element Vector{Float64}:
 0.9344218969345093
 0.8561021089553833
 0.6830916404724121
 0.39803004264831543
 0.6795381307601929
 0.7997522354125977

[mfherbst]

@CedricTravelletti Correct me if I'm wrong, but I believe the current state is that this is stalled until the Lux model state PR goes in (which should hopefully be very soon).