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

Utility to access optional AtomsBase keys (e.g. charge) #78

Open
ejmeitz opened this issue Jul 3, 2023 · 2 comments
Open

Utility to access optional AtomsBase keys (e.g. charge) #78

ejmeitz opened this issue Jul 3, 2023 · 2 comments

Comments

@ejmeitz
Copy link
Contributor

ejmeitz commented Jul 3, 2023

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, )
    if hasatomkey(system, x)
        if x  atomsbase_keys
            return getproperty(AtomsBase, x)(system, i)
        elseif x  custom_keys
            return getproperty(Molly, x)(system, i)
        end
    else
      throw(KeyError("Key $(x) not present in system."))
    end
end
@cortner
Copy link
Member

cortner commented May 18, 2024

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.

@cortner
Copy link
Member

cortner commented Jul 1, 2024

@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.

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

No branches or pull requests

2 participants