You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue is in reference to the solution to #74 and possible ways to improve the use of optional keys with other parts of the AtomsBase interface.
The original problem: Defining a getindex(sys::AbstractSystem, i::Integer, x::Symbol) for libraries (like Molly) that make use of the AtomView object can be challenging especially when optional atomkeys like charge are used.
The current solution to implement the getindex function in Molly is pasted below. The use of optional keys makes implementing this function tricky. For example, Molly now has to maintain the custom_keys list and also define functions that mimic the AtomsBase interface to retrieve the property specified by x if x is optional in the AtomsBase interface. For mandatory keys like atomic_number, AtomsBase requires the definition of AtomsBase.atomic_number but that is not the case for optional keys (e.g. you cannot define AtomsBase.charge). In general, it would be nice to have a method to access optional keys through the AtomsBase interface instead of relying on code custom to the library implementing AtomsBase.
One thought I had was maybe add all of the optional keys to the interface but set the methods to nothing. Not sure if this does what I think it does, but I think this would make the function optional to overload with your own definition and if you did not overload it gets compiled to a no-op.
Something like: charge(sys::AbstractSystem) = nothing
Might be missing something from this explanation so feel free to add anything @mfherbst.
function Base.getindex(system::Union{System, ReplicaSystem}, i, x::Symbol)
atomsbase_keys = (:position, :velocity, :atomic_mass, :atomic_number)
custom_keys = (:charge, )
ifhasatomkey(system, x)
if x ∈ atomsbase_keys
returngetproperty(AtomsBase, x)(system, i)
elseif x ∈ custom_keys
returngetproperty(Molly, x)(system, i)
endelsethrow(KeyError("Key $(x) not present in system."))
endend
The text was updated successfully, but these errors were encountered:
In the next 1-2 weeks I will produce an example how to create much more flexible systems using DecoratedParticles.jl or some to be developed variant of this package. This will allow fast access to particles that have arbitrary additional properties or features. I think it then makes perfect sense to introduce a range of additional accessor functions.
@ejmeitz -- please take a look at DecoratedParticles.jl, it can easily do what you want. At the moment it is still a little experimental but as soon as there are users other than my own group I can be convinced of settling some interface and start providing backward compatibility.
This issue is in reference to the solution to #74 and possible ways to improve the use of optional keys with other parts of the AtomsBase interface.
The original problem: Defining a
getindex(sys::AbstractSystem, i::Integer, x::Symbol)
for libraries (like Molly) that make use of the AtomView object can be challenging especially when optionalatomkeys
likecharge
are used.The current solution to implement the
getindex
function in Molly is pasted below. The use of optional keys makes implementing this function tricky. For example, Molly now has to maintain thecustom_keys
list and also define functions that mimic the AtomsBase interface to retrieve the property specified byx
ifx
is optional in the AtomsBase interface. For mandatory keys like atomic_number, AtomsBase requires the definition ofAtomsBase.atomic_number
but that is not the case for optional keys (e.g. you cannot define AtomsBase.charge). In general, it would be nice to have a method to access optional keys through the AtomsBase interface instead of relying on code custom to the library implementing AtomsBase.One thought I had was maybe add all of the optional keys to the interface but set the methods to nothing. Not sure if this does what I think it does, but I think this would make the function optional to overload with your own definition and if you did not overload it gets compiled to a no-op.
Something like:
charge(sys::AbstractSystem) = nothing
Might be missing something from this explanation so feel free to add anything @mfherbst.
The text was updated successfully, but these errors were encountered: