Gregstrq / LightLattices.jl

Lazy interface to lattices with arbitrary unit cells.
MIT License
4 stars 0 forks source link

field names somewhat counter to convention #4

Open rkurchin opened 3 years ago

rkurchin commented 3 years ago

In RegularLattice, these are some of the fields:

    """
    The number of basis cells along each of the basis directions.
    """
    lattice_dims::NTuple{D, Int}
    """
    Coordinates of the basis vectors of the underlying Bravais lattice. `basis[:, i]` gives the ``i``-th basis vector.
    """
    basis::SMatrix{D,D, T}
    """
    Unit cell of the lattice.
    """
    unit_cell::CT

And here is the relevant bit of signature of the constructor:

    #Arguments
    - `lattice_dims::NTuple{D,Int}`: specifies the number of unit cells along each of the basis directions.
    - `basis::SMatrix{D,D,T}`: stores as columns the basis vectors of the underlying Bravais lattice.
    - `unit_cell::AbstractCell{D,T}`: specifies the unit cell of the lattice; if the unit cell does not have any special structure, an instance of `TrivialCell` should be used.
    """

However, some of this terminology is a bit counter to convention (see e.g. this). In particular, in crystallography, we think of a crystal as being composed of a (Bravais) lattice (which we can specify purely by the three primitive cell translation vectors) and a basis, i.e. which atoms occupy a single unit cell and where.

To "fully" resolve this, the package and types would need to refer to crystals rather than lattice, but I don't personally think such a dramatic change is necessary. Since there are people other than crystallographers that use lattices, I don't think it's reasonable to ask that the terminology of this package be fully consistent with crystallographic terminology. But it would probably be nice if it were not directly counter to it. 😉

My suggestion would be to remame basis to something like primtive_cell. This is also nice because it distinguishes from what I think of as a "supercell" (repeating tiling of primitive cell), and you specify as lattice_dims. As far as what to call unit_cell, it's perhaps too confusing to suggest the crystallographic term of basis, but I also don't have any other great ideas right now...

Gregstrq commented 3 years ago

What do you think about that?

rkurchin commented 3 years ago

Sounds like an improvement to me!

Gregstrq commented 3 years ago

I gave it a bit more thought. With unit_cell, it is important to keep cell in the name, so it can be renamed to basis_cell. primitive_vecs is a good name so it stays.

On the other hand, I am not so sure about using supercell_dims. I think the term supercell might not be familiar to the general audience who just needs some lattice implementation. Also, for some reason, I have completely different association with this term. One can describe the same crystal as a combination of different basis and set of primitive vectors. As such, one can stack several basis cells together to use as a new basis cell. I believe it is called supercell in this context.

rkurchin commented 3 years ago

That's fair. Of course it's your package and so you get the final say. I think this sounds sensible, and still definitely an improvement in my eyes over the current state of things! Thanks for your openness to feedback! :)

ederag commented 2 years ago

IMO "basis" should never be used for what is also called "motif", since in linear-algebra terms the set of primitive translation vectors do form a basis of the direct space, and a basis of the lattice (the lattice is generated by the set of all possible integer components, in this basis). So I'd rather support keeping

    """
    Coordinates of the basis vectors of the underlying Bravais lattice.
    `basis[:, i]` gives the ``i``-th basis vector.
    """
    basis::SMatrix{D,D, T}