Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update constructor for positions and lattice in Model and basis #675

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update constructor for positions and lattice in Model and basis #675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63ba2ec
Update constructor for positions and lattice
mfherbst Jun 27, 2022
f84945b
empty
mfherbst Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Model.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,26 @@ function Model(lattice::AbstractMatrix{<:Quantity}, atoms::Vector{<:Element},
Model(austrip.(lattice), atoms, positions; kwargs...)
end

"""
Model(model, lattice, positions)
Model(model, lattice)
Model(model, positions)

Construct an identical model to `model` with the option to change `lattice` or `positions`
(useful for geometry or lattice optimisations).
"""
function Model(model::Model{T}, lattice::AbstractMatrix{U},
mfherbst marked this conversation as resolved.
Show resolved Hide resolved
positions::Vector{<:AbstractVector}=model.positions) where {T, U}
TT = promote_type(T, U, eltype(positions[1]))
Model(TT.(lattice), model.atoms, positions;
model.model_name, model.n_electrons, magnetic_moments=[], terms=model.term_types,
model.temperature, model.smearing, model.spin_polarization, model.symmetries)
end
function Model(model::Model, positions::Vector{<:AbstractVector})
Model(model, model.lattice, positions)
end


normalize_magnetic_moment(::Nothing)::Vec3{Float64} = (0, 0, 0)
normalize_magnetic_moment(mm::Number)::Vec3{Float64} = (0, 0, mm)
normalize_magnetic_moment(mm::AbstractVector)::Vec3{Float64} = mm
Expand Down
21 changes: 21 additions & 0 deletions src/PlaneWaveBasis.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,27 @@ Creates a new basis identical to `basis`, but with a custom set of kpoints
basis.symmetries_respect_rgrid, basis.comm_kpts)
end

"""
PlaneWaveBasis(basis, lattice, positions)
PlaneWaveBasis(basis, lattice)
PlaneWaveBasis(basis, positions)

Creates a new basis, identical to `basis`, but different `lattice` or `positions`.
"""
function PlaneWaveBasis(basis::PlaneWaveBasis, lattice::AbstractMatrix,
Copy link
Member

Choose a reason for hiding this comment

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

How about just PlaneWaveBasis(basis, model)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Not so sure. My idea was to have a way to update position and lattice really from a high level with one function and exactly not needing to go through the separate construction of model and basis "by hand".

positions::Vector{<:AbstractVector}=basis.model.positions)
model = Model(basis.model, lattice, positions)
PlaneWaveBasis(model,
basis.Ecut, basis.fft_size, basis.variational,
basis.kcoords_global, basis.kweights_global,
basis.kgrid, basis.kshift, basis.symmetries_respect_rgrid,
basis.comm_kpts)
Copy link
Member

Choose a reason for hiding this comment

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

Hm this can have unintended consequences, ie the model can change symmetries, lattice leading to different fft_size, etc...

Copy link
Member

Choose a reason for hiding this comment

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

There are two possible intended behaviors here: either keep the basis as much the same as possible, or keep it as close to a new basis as possible. This sort of does neither. Not sure what to do here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it should be as close to the new basis as possible, i.e. it should be a new consistent basis with the new structure. So yes, I think this should change the FFT size if needed. I did not think about that.

Copy link
Member

Choose a reason for hiding this comment

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

Then I'm not sure this can be done sensibly, because we have different ways of constructing a pwbasis. I think we should move towards encapsulating the kpoint grid (and possibly the G grid ?) better,and then have a unique pwbasis(::Model, ::Kgrid) constructor, which would then make what you want to do here more straightforward.

Copy link
Member

Choose a reason for hiding this comment

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

By Kgrid I mean the specification (uniform/custom/symmetrized/shifted/..., with symmetries or not, not the actual instantiation) of the kgrid. Same for the G grid (fft_size, variational, )

end
function PlaneWaveBasis(basis::PlaneWaveBasis, positions::Vector{<:AbstractVector})
PlaneWaveBasis(basis, basis.model.lattice, positions)
end


"""
G_vectors(fft_size::Tuple)

Expand Down
17 changes: 2 additions & 15 deletions src/postprocess/stresses.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,8 @@ Compute the stresses (= 1/Vol dE/d(M*lattice), taken at M=I) of an obtained SCF
# TODO optimize by only computing derivatives wrt 6 independent parameters
# compute the Hellmann-Feynman energy (with fixed ψ/occ/ρ)
function HF_energy(lattice::AbstractMatrix{T}) where T
basis = scfres.basis
model = basis.model
new_model = Model(lattice, model.atoms, model.positions;
model.n_electrons,
magnetic_moments=[], # not used because symmetries explicitly given
terms=model.term_types,
model.temperature,
model.smearing,
model.spin_polarization,
model.symmetries)
new_basis = PlaneWaveBasis(new_model,
basis.Ecut, basis.fft_size, basis.variational,
basis.kcoords_global, basis.kweights_global,
basis.kgrid, basis.kshift, basis.symmetries_respect_rgrid,
basis.comm_kpts)
# Update lattice (also changes floating-point type)
new_basis = PlaneWaveBasis(scfres.basis, lattice)
ρ = DFTK.compute_density(new_basis, scfres.ψ, scfres.occupation)
energies, _ = energy_hamiltonian(new_basis, scfres.ψ, scfres.occupation;
ρ, scfres.eigenvalues, scfres.εF)
Expand Down