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.

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

lattice(lat; kw...) #95

Merged
merged 3 commits into from
Sep 21, 2020
Merged

lattice(lat; kw...) #95

merged 3 commits into from
Sep 21, 2020

Conversation

pablosanjose
Copy link
Owner

As mentioned in the cancelled #93 we need to complete the ways we provide to change a lattice, or to build one lattice from another. At the moment we cannot change the Bravais matrix, the type, the embedding dimension or the sublattice names of a lattice after creation. This PR, that builds on #94, does just that. It provides a method lattice(lat; kw...) where kw are any of the kwargs for lattice creation (bravais, dim, type and names). To do this elegantly we needed to move bravais to a kwarg, which was done in #94.

From the implementation point of view, the key of this PR is to keep the mutation as efficient as possible. If we're only changing Bravais vectors, there is no need to touch the site position array or sublattice offsets. In contrast, if we're changing the dimension or the position type we need to allocate a whole new Unitcell. This done via parametric dispatch.

An open question here is whether we want also a hamiltonian(ham; kw...) or not. The keywords of hamiltonian are orbitals and type. Changing any of the two (unless only change the orbital names, but not their number, which is kind of a trivial change) implies recomputing the Hamiltonian (i.e. reapplying the model to the lattice, which often does not make sense without changing the model itself), so I'm inclined not to add such a method.

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2020

Codecov Report

Merging #95 into master will increase coverage by 0.20%.
The diff coverage is 84.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   62.15%   62.36%   +0.20%     
==========================================
  Files          16       16              
  Lines        2603     2617      +14     
==========================================
+ Hits         1618     1632      +14     
  Misses        985      985              
Impacted Files Coverage Δ
src/convert.jl 53.84% <20.00%> (+3.84%) ⬆️
src/hamiltonian.jl 74.74% <100.00%> (ø)
src/lattice.jl 81.42% <100.00%> (+1.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37b2dba...3593331. Read the comment docs.

@pablosanjose
Copy link
Owner Author

Another future evolution of this is to finally implement Base.push!(lattice, site), which we currently don't have. Since that doesn't change the lattice type, it should be a simple matter of splicing the site (or sites) and recomputing the sublat offsets. And then, we can also consider merge!(lattice, sublattices)

@pablosanjose pablosanjose merged commit 4d2acc1 into master Sep 21, 2020
@pablosanjose pablosanjose deleted the modify branch September 21, 2020 14:48
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