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

Avoid all use of deepcopy #66

Merged
merged 1 commit into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/ReplicaExchangeMC.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function replica_exchange!(replica::Replica)
S_curr = running_energy(replica.sampler) / get_temp(replica.sampler)

# Backup current configuration
backup_spins = deepcopy(replica.sampler.sys.dipoles)
backup_spins = copy(replica.sampler.sys.dipoles)

# Swap trial configuration with partner
MPI.Sendrecv!(
Expand Down
6 changes: 6 additions & 0 deletions src/System/Ewald.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ function Ewald(sys::System{N}) where N
return Ewald(A, ϕ, FA, Fs, Fϕ, plan, ift_plan)
end

# Clone all mutable state within Ewald. Note that `A`, `FA`, and plans should be
# immutable data.
function clone_ewald(ewald::Ewald)
(; A, ϕ, FA, Fs, Fϕ, plan, ift_plan) = ewald
return Ewald(A, copy(ϕ), FA, copy(Fs), copy(Fϕ), plan, ift_plan)
end

# Tensor product of 3-vectors
(⊗)(a::Vec3,b::Vec3) = reshape(kron(a,b), 3, 3)
Expand Down
15 changes: 12 additions & 3 deletions src/System/Interactions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ function empty_interactions(nb, N)
end
end

# Creates a clone of the lists of exchange interactions, which can be mutably
# updated.
function clone_interactions(ints::Interactions)
(; aniso, heisen, exchange, biquad) = ints
return Interactions(aniso, copy(heisen), copy(exchange), copy(biquad))
end

function interactions_homog(sys::System{N}) where N
return sys.interactions_union :: Vector{Interactions}
end
Expand All @@ -33,11 +40,13 @@ function to_inhomogeneous(sys::System{N}) where N
is_homogeneous(sys) || error("System is already inhomogeneous.")
ints = interactions_homog(sys)

ret = deepcopy(sys)
ret = clone_system(sys)
nb = nbasis(ret.crystal)
ret.interactions_union = Array{Interactions}(undef, ret.latsize..., nb)
for cell in all_cells(ret)
ret.interactions_union[cell, :] = deepcopy(ints)
for b in 1:nbasis(ret.crystal)
for cell in all_cells(ret)
ret.interactions_union[cell, b] = clone_interactions(ints[b])
end
end

return ret
Expand Down
32 changes: 27 additions & 5 deletions src/System/System.jl
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,33 @@ function Base.show(io::IO, ::MIME"text/plain", sys::System{N}) where N
end
end

# Per Julia developers, `deepcopy` is memory unsafe, especially in conjunction
# with C libraries. We were observing very confusing crashes that surfaced in
# the FFTW library, https://github.com/JuliaLang/julia/issues/48722. To prevent
# this from happening again, avoid all uses of `deepcopy`, and create our own
# stack of `clone` functions instead.
Base.deepcopy(_::System) = error("Use `clone_system` instead of `deepcopy`.")

# Creates a clone of the system where all the mutable internal data is copied.
# It should be thread-safe to use the original and the copied systems, without
# any restrictions.
function clone_system(sys::System{N}) where N
(; origin, mode, crystal, latsize, Ns, gs, κs, extfield, interactions_union, ewald, dipoles, coherents, units, rng) = sys

origin_clone = isnothing(origin) ? nothing : clone_system(origin)
ewald_clone = isnothing(ewald) ? nothing : clone_ewald(ewald)

# Dynamically dispatch to the correct `map` function for either homogeneous
# (Vector) or inhomogeneous interactions (4D Array)
interactions_clone = map(clone_interactions, interactions_union)

# Empty buffers are required for thread safety.
empty_dipole_buffers = Array{Vec3, 4}[]
empty_coherent_buffers = Array{CVec{N}, 4}[]

function clone_spin_state(sys::System{N}) where N
System(sys.origin, sys.mode, sys.crystal, sys.latsize, sys.Ns, sys.gs, sys.κs, sys.extfield,
sys.interactions_union, sys.ewald, copy(sys.dipoles), copy(sys.coherents),
sys.dipole_buffers, sys.coherent_buffers, sys.units, copy(sys.rng))
System(origin_clone, mode, crystal, latsize, Ns, copy(gs), copy(κs), copy(extfield),
interactions_clone, ewald_clone, copy(dipoles), copy(coherents),
empty_dipole_buffers, empty_coherent_buffers, units, copy(rng))
end


Expand Down Expand Up @@ -362,7 +384,7 @@ function reshape_geometry_aux(sys::System{N}, new_latsize::NTuple{3, Int}, new_c
# reshapings, `sys.origin` keeps its original meaning. Make a deep copy so
# that the new system fully owns `origin`, and mutable updates to the
# previous system won't affect this one.
origin = deepcopy(isnothing(sys.origin) ? sys : sys.origin)
origin = clone_system(isnothing(sys.origin) ? sys : sys.origin)

# If `new_cell_size == I`, we can effectively restore the unit cell of
# `origin`, but with `new_latsize`
Expand Down
2 changes: 1 addition & 1 deletion test/mc_tests/REMC_H_Ising2D.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ replica = Replica(IsingSampler(system, kT, 1), α)
# the sampling distribution or the system
function set_α!(replica::Replica, α::Float64)
replica.α = α
copy_sites = deepcopy(replica.sampler.system.sites)
copy_sites = copy(replica.sampler.system.sites)
replica.sampler.system = create_system(α)
replica.sampler.system.sites .= copy_sites
reset_running_energy!(replica.sampler)
Expand Down
23 changes: 13 additions & 10 deletions test/test_energy_consistency.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
add_linear_interactions!(sys, mode)
add_quadratic_interactions!(sys, mode)
add_quartic_interactions!(sys, mode)
# enable_dipole_dipole!(sys) # workaround segfault
enable_dipole_dipole!(sys)

# Random field
for idx in Sunny.all_sites(sys)
Expand All @@ -18,16 +18,19 @@
rand!(sys.rng, sys.κs)
# Random spins
randomize_spins!(sys)
# Make inhomogeneous
if inhomog
sys = to_inhomogeneous(sys)
set_vacancy_at!(sys, (1,1,1,1))
set_exchange_at!(sys, 0.5, Bond(1, 2, [1,0,0]), (1,1,1,1))
set_biquadratic_at!(sys, 0.7, Bond(2, 3, [0,-1,0]), (3,1,1,2))
set_anisotropy_at!(sys, 0.4*(𝒮[1]^4+𝒮[2]^4+𝒮[3]^4), (2,2,2,4))
end

return sys
if !inhomog
return sys
else
# Add some inhomogeneous interactions
sys2 = to_inhomogeneous(sys)
@test energy(sys2) ≈ energy(sys)
set_vacancy_at!(sys2, (1,1,1,1))
set_exchange_at!(sys2, 0.5, Bond(1, 2, [1,0,0]), (1,1,1,1))
set_biquadratic_at!(sys2, 0.7, Bond(2, 3, [0,-1,0]), (3,1,1,2))
set_anisotropy_at!(sys2, 0.4*(𝒮[1]^4+𝒮[2]^4+𝒮[3]^4), (2,2,2,4))
return sys2
end
end


Expand Down