Atom interface definition is not documented

[tjjarvinen]

The interface for atoms is not currently documented. Only system interface is.

The interface defines that Base.getindex(sys,i) has to exist, but it does not define what you can do with the atoms you get as return.

The default AtomsBase systems use Atom and AtomView, which in a way form the standard interface, but that has not been documented anywhere.

There are notable differences on how you use atoms position(atom) and systems position(system, i). Which can cause confusion for the users.

[jgreener64]

A complication here is that if we require the likes of position(atom) then atom has to be a custom type to avoid type piracy. So a simple system type that contains arrays for coordinates/elements etc. has to have a wrapper type for atoms as well, even if it is unlikely to be used (e.g. on GPU).

The alternative is to remove the requirement for the system to be indexable.

Of course the default AtomsBase systems can define position(atom) etc. anyway, since they have the required atom types. But it's a tradeoff as to whether it is worth it in the general case of making a system compliant with the interface.

[cortner]

Why would AtomsBase.position(::SVector{3, T}) constitute type piracy? (or maybe I misunderstand the point)

[jgreener64]

Because a package that defines an arbitrary system type compliant with the interface is separate from both AtomsBase and StaticArrays.

(Technically we use Base.position currently but the point applies more generally to mass, species etc.)

[jgreener64]

You could put AtomsBase.position(::SVector{3, T}) in AtomsBase but that means defining iteration of a system to mean returning the coordinates, and then it is unclear how to access the other properties.

[cortner]

I still don't get it.

AtomsBase.position(r::SVector) = r 

doesn't interfere with anything else, especially not with how you access a system. To be clear I am NOT proposing to define

AtomsBase.position(r::SVector, i) = r[i]

That would be terrible and if that was your point then I agree with you. (Formally it doesn't contradict anything either but very likely lead to confusion.)

[jgreener64]

AtomsBase.position(r::SVector) = r is type piracy if defined in a package that isn't AtomsBase or StaticArrays. It can lead to unexpected behaviour on importing the defining package.

That definition could go into AtomsBase without issue, but in the context of how Teemu is interpreting the interface above if the SVector comes from indexing the system then we are effectively saying that iterating a system returns the positions.

[cortner]

Ok then there is no confusion it would of course have to be in AtomsBase. I don't have a problem with that.

[cortner]

I now understand the issue with Teemus proposal, thanks for clarifying.