AtomsBase.jl icon indicating copy to clipboard operation
AtomsBase.jl copied to clipboard

Organizing boundary conditions

Open Gregstrq opened this issue 4 years ago • 7 comments

Right now, the boundary conditions are specified as AbstractVector{BoundaryCondition}.

I suggest we make it NTuple{D, BoundaryCondition} (D is the dimensionality of the system):

  • Tuple is covariant in its type parameters: Tuple{Periodic, Periodic, DirichletZero}is a subtype ofNTuple{3, BoundaryCondition}`.
  • Widely used variants of boundary condition, such as Periodic, DirichletZero, are singleton types (they don't have any fields). So, if the boundary conditions are specified as for example Tuple{Periodic,Periodic,DirichletZero}, you don't actually need to store this tuple, you can directly dispatch on its type.
  • If we also define the system as AbstractSystem{D}, than we enforce that the size of the tuple of boundary conditions equals to the dimensionality of the system.

Gregstrq avatar Oct 05 '21 13:10 Gregstrq

I think this makes sense. The last bullet point would also allow checks on dimensionality of positions/velocities. Not sure it makes sense to do that at the top level since people could conceivably want fat or tall matrices, but would be easy to put into a concrete implementation.

rkurchin avatar Oct 05 '21 15:10 rkurchin

I think dimensionality of the system is still a well-defined and clear concept irrespective of whether the users want to stack the coordinates as columns or rows of a matrix.

Gregstrq avatar Oct 05 '21 16:10 Gregstrq

Oh yeah for sure, sorry, I realize now that was unclear. I just meant it doesn't necessarily make sense to have a check on the shape of the matrices coded in at the top level for that reason. I agree with the general proposal!

rkurchin avatar Oct 05 '21 16:10 rkurchin

Sorry, but what do you mean by the matrices coded at the top level? (I thought you were talking about a matrix of all the coordinates or a matrix of all the velocities)

Gregstrq avatar Oct 05 '21 16:10 Gregstrq

Yeah, that is what I meant. I'm just saying since some people may want (e.g. in 3D) 3 x N or N x 3 (or AoS), we don't need to also impose a check that e.g. the first dimension of the positions array is D or something like that at the level of the abstract dispatches, but that may be something that developers of concrete implementations would want, and having the type parameter D accessible will make that easier. I'm agreeing with you, just in a long-winded and confusing way. 😆

rkurchin avatar Oct 05 '21 18:10 rkurchin

BC-related comments from @cortner from #23 :

re Periodic...

should this have a flag true/false? Since the bc is implemented as a vector (or tuple) of BCs, you probably want each element of that vector to be e.g. a Periodic() . But now suppose you want a system that is open in the x, y direction and periodic in the z direction (e.g. dislocations). Then you need a different type Open() and you have a type instability. But with Periodic(true), Periodic(false) there is no problem.

Personally, I don't really see how one needs anything other than a boolean here? I can see many other options such as reflecting, embedding, etc but those are not really relevant for the particle system per se, but rather for fields in the background, or for the embedding one would need additional structures anyhow into which to embed the System...

But I don't see any harm in keeping the Periodic type.

rkurchin avatar Nov 17 '21 18:11 rkurchin

also see #97 and #99 -- apologies for duplicating discussions.

cortner avatar May 16 '24 04:05 cortner