Skip to content

Commit

Permalink
Fix StackOverFlowError when constructing from Vector{Any} (#38)
Browse files Browse the repository at this point in the history
  • Loading branch information
mfherbst authored Mar 15, 2022
1 parent 0e2e587 commit b7a5a19
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "AtomsBase"
uuid = "a963bdd2-2df7-4f54-a1ee-49d51e6be12a"
authors = ["JuliaMolSim community"]
version = "0.2.0"
version = "0.2.1"

[deps]
PeriodicTable = "7b2266bf-644c-5ea3-82d8-af4bbd25a884"
Expand Down
16 changes: 11 additions & 5 deletions src/atom.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
#
export Atom, atomic_system, periodic_system, isolated_system

# Valid types for atom identifiers
const AtomId = Union{Symbol,AbstractString,Integer}

struct Atom{D, L<:Unitful.Length, V<:Unitful.Velocity, M<:Unitful.Mass}
position::SVector{D, L}
velocity::SVector{D, V}
Expand All @@ -29,7 +32,7 @@ function Base.propertynames(at::Atom, private::Bool=false)
end
end

function Atom(identifier::Union{Symbol,AbstractString,Integer},
function Atom(identifier::AtomId,
position::AbstractVector{L},
velocity::AbstractVector{V}=zeros(3)u"bohr/s";
atomic_symbol=Symbol(elements[identifier].symbol),
Expand All @@ -39,7 +42,6 @@ function Atom(identifier::Union{Symbol,AbstractString,Integer},
Atom{length(position), L, V, M}(position, velocity, atomic_symbol,
atomic_number, atomic_mass, Dict(kwargs...))
end
Atom(id_pos::Pair; kwargs...) = Atom(id_pos...; kwargs...)

# Update constructor: Amend any atom by extra data.
function Atom(;atom, kwargs...)
Expand All @@ -52,18 +54,22 @@ function Atom(;atom, kwargs...)
end
Atom(atom::Atom; kwargs...) = Atom(; atom, kwargs...)

function Base.convert(::Type{Atom}, id_pos::Pair{<:AtomId,<:AbstractVector{<:Unitful.Length}})
Atom(id_pos...)
end

#
# Special high-level functions to construct atomic systems
#
atomic_system(atoms::AbstractVector{<:Atom}, box, bcs) = FlexibleSystem(atoms, box, bcs)
atomic_system(atoms::AbstractVector, box, bcs) = atomic_system(Atom.(atoms), box, bcs)
atomic_system(atoms::AbstractVector, box, bcs) = FlexibleSystem(convert.(Atom, atoms), box, bcs)

function isolated_system(atoms::AbstractVector{<:Atom})
# Use dummy box and boundary conditions
D = n_dimensions(first(atoms))
atomic_system(atoms, infinite_box(D), fill(DirichletZero(), D))
end
isolated_system(atoms::AbstractVector) = isolated_system(Atom.(atoms))
isolated_system(atoms::AbstractVector) = isolated_system(convert.(Atom, atoms))

function periodic_system(atoms::AbstractVector,
box::AbstractVector{<:AbstractVector},
Expand All @@ -73,7 +79,7 @@ function periodic_system(atoms::AbstractVector,
!fractional && return atomic_system(atoms, box, boundary_conditions)

parse_fractional(atom::Atom) = atom
function parse_fractional(atom::Pair)
function parse_fractional(atom::Pair)::Atom
id, pos_fractional = atom
Atom(id, lattice * pos_fractional)
end
Expand Down
15 changes: 15 additions & 0 deletions test/atom.jl
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,19 @@ using Test
@test boundary_conditions(system) == [Periodic(), Periodic(), Periodic()]
@test position(system) == [[0.0, -0.625, 0.0], [1.25, 0.0, 0.0], [0.0, 0.0, 0.0]]u"Å"
end


@testset "no stackoverflow" begin
box = [[10, 0.0, 0.0], [0.0, 5, 0.0], [0.0, 0.0, 7]]u"Å"
atoms = Any[:Si => [0.0, -0.125, 0.0],
:C => [0.125, 0.0, 0.0],
Atom(:H, zeros(3) * u"Å")]
system = periodic_system(atoms, box; fractional=true)
@test length(system) == 3

system = isolated_system(Any[:Si => [0.0, 1.0, 1.5]u"Å",
:C => [0.0, 0.8, 1.7]u"Å",
Atom(:H, zeros(3) * u"Å")])
@test length(system) == 3
end
end
4 changes: 2 additions & 2 deletions test/fast_system.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ using PeriodicTable
@testset "Fast system" begin
box = [[1, 0, 0], [0, 1, 0], [0, 0, 1]]u"m"
bcs = [Periodic(), Periodic(), DirichletZero()]
atoms = Atom.([:C => [0.25, 0.25, 0.25]u"m",
:C => [0.75, 0.75, 0.75]u"m"])
atoms = Atom[:C => [0.25, 0.25, 0.25]u"m",
:C => [0.75, 0.75, 0.75]u"m"]
system = FastSystem(atoms, box, bcs)

@test length(system) == 2
Expand Down

2 comments on commit b7a5a19

@mfherbst
Copy link
Member Author

Choose a reason for hiding this comment

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

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/56667

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.2.1 -m "<description of version>" b7a5a199f60d58feb9991a1d5ac936c509c06ecd
git push origin v0.2.1

Please sign in to comment.