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

Fix in vlplot: make CurrentShader and DensityShader work with multiple states #141

Merged
merged 2 commits into from
Nov 25, 2020

Conversation

pablosanjose
Copy link
Owner

This completes #140 by adding support for multiple states in current and density shaders. Thanks to @fernandopenaranda for the heads up!

This PR also tries to make CurrentShader more correct in open systems. In doing so I realized we have a serious problem here: we cannot compute currents across unit cell boundaries from the Bloch eigenstate alone. We also need its Bloch phases. Solving this issue requires some thought about design (e.g. what information should we store and provide about a generic eigenstate?), so I do not attempt to fix that here. For the moment, therefore, be aware that current density plots is not going to be correct in open systems.

This also introduces a stopgap fix for a problem also reported by @fernandopenaranda:

julia> h = LatticePresets.honeycomb() |> hamiltonian(hopping(I)) |> unitcell(10, region = !RP.circle(2, (0,6)));

julia> bs = bandstructure(h, subticks = 13);
Step 1/2 - Diagonalising: 100%|███████████████████████████████████████████████████████████████████████████████████████████████████| Time: 0:00:01
Step 2/2 - Knitting bands: 100%|██████████████████████████████████████████████████████████████████████████████████████████████████| Time: 0:00:02
ERROR: BoundsError: attempt to access 12×12 Array{UnitRange{Int64},2} at index [13, 1]

It it not worthwhile to dig deeper into this issue to make a better fix than the one included here, as there is a pending refactor of the bandstructure machinery that will make the corresponding code obsolete

@codecov-io
Copy link

codecov-io commented Nov 24, 2020

Codecov Report

Merging #141 (5433953) into master (98016ec) will increase coverage by 0.10%.
The diff coverage is 31.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage   61.03%   61.13%   +0.10%     
==========================================
  Files          16       16              
  Lines        2972     2972              
==========================================
+ Hits         1814     1817       +3     
+ Misses       1158     1155       -3     
Impacted Files Coverage Δ
src/plot_vegalite.jl 0.00% <0.00%> (ø)
src/tools.jl 36.84% <0.00%> (-0.17%) ⬇️
src/hamiltonian.jl 80.00% <66.66%> (-0.31%) ⬇️
src/bandstructure.jl 81.88% <100.00%> (+0.09%) ⬆️

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 d9f6a7d...a04c78c. Read the comment docs.

@pablosanjose pablosanjose merged commit d8179f2 into master Nov 25, 2020
@pablosanjose pablosanjose deleted the fixshaders branch November 25, 2020 16:52
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