search

JuliaMolSim / DFTK.jl

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

`compute_bands` doesn't work for a non-atomic system in 3D. #976

Open JaydevSR opened 3 months ago

JaydevSR commented 3 months ago

For a non-atomic system, i.e., atoms and positions vectors are empty for the model, the band computation using compute_bands gives the following error:

ERROR: LoadError: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
Stacktrace:
  [1] reduce_empty(op::Base.MappingRF{DFTK.var"#909#915", Base.BottomRF{typeof(Base.add_sum)}}, ::Type{Any})
    @ Base .\reduce.jl:361
  [2] reduce_empty_iter
    @ .\reduce.jl:384 [inlined]
  [3] reduce_empty_iter
    @ .\reduce.jl:383 [inlined]
  [4] foldl_impl
    @ .\reduce.jl:49 [inlined]
  [5] mapfoldl_impl
    @ .\reduce.jl:44 [inlined]
  [6] mapfoldl
    @ .\reduce.jl:175 [inlined]
  [7] mapreduce
    @ .\reduce.jl:307 [inlined]
  [8] sum
    @ .\reduce.jl:535 [inlined]
  [9] parse_system(system::AtomsBase.FlexibleSystem{3, Any, Quantity{Float64, 𝐋, Unitful.FreeUnits{(a₀,), 
𝐋, nothing}}} )
    @ DFTK D:\Projects\OpenSource\DFTK.jl\src\external\atomsbase.jl:48
 [10] spglib_cell(system::AtomsBase.FlexibleSystem{3, Any, Quantity{Float64, 𝐋, Unitful.FreeUnits{(a₀,), � 
,, nothing}}})
    @ DFTK D:\Projects\OpenSource\DFTK.jl\src\external\spglib.jl:31
 [11] spglib_cell(model::Model{Float64, Float64}, magnetic_moments::Vector{Any})
    @ DFTK D:\Projects\OpenSource\DFTK.jl\src\external\spglib.jl:35
 [12] irrfbz_path(model::Model{Float64, Float64}, magnetic_moments::Vector{Any}; dim::Int64, space_group_number::Int64)
    @ DFTK D:\Projects\OpenSource\DFTK.jl\src\postprocess\band_structure.jl:140
 [13] irrfbz_path
    @ D:\Projects\OpenSource\DFTK.jl\src\postprocess\band_structure.jl:106 [inlined]
 [14] #compute_bands#968
    @ D:\Projects\OpenSource\DFTK.jl\src\postprocess\band_structure.jl:79 [inlined]
 [15] compute_bands(scfres::@NamedTuple{…})
    @ DFTK D:\Projects\OpenSource\DFTK.jl\src\postprocess\band_structure.jl:78
 [16] top-level scope
    @ REPL[33]:1
in expression starting at REPL[33]:1
Some type information was truncated. Use `show(err)` to see complete types.

The main problem is that there are multiple places where reduction over atoms is performed without passing an initial value. Particularly, even if the line src\external\atomsbase.jl:48 is changed to provide a value to the init kwarg, there are places in Spglib.jl and Brillouin.jl where the same problem occurs.

After discussing with @mfherbst over slack, other than making changes wherever reduction happens, one possible fix would be to short circuit for the case of no atoms where there's no need for creation of an spglib cell.

Also, a quick workaround is to define dummy atoms without adding them to the Hamiltonian, but it's very hacky.

mfherbst commented 3 months ago

Thanks @JaydevSR.

I think parse_system is probably an important one to fix, that gets used a lot. Otherwise I think a helpful message in irrfbz_path for the case that dim > 1 and no atoms are given should cover most cases, I believe. Also I suppose adding a unit test to ensure we don't again trigger such a bug in the future would be useful.

mfherbst commented 3 months ago

Oh and having looked at the code for 1s: I think if you actually manually supply the kpath it should work in 3D, but I might overlook something.