search

materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.51k stars 863 forks source link

what happened to LammpsData.from_structure classmethod? #900

Closed matk86 closed 6 years ago

matk86 commented 6 years ago

it used to be there until the recent PR. @adengz why did you remove it? it is very useful to have it in there for generating data files for general structure objects.

adengz commented 6 years ago

Sorry it was removed in the recent PR because

  1. A LAMMPS data file usually contains more information than a structure does.
  2. There is no reason to place the molecule in the center of the simulation box, or alter the simulation box size simply based the size of input molecule.

To work with pmg objects like structure or molecule, use the new Topology and ForceField objects.

shyuep commented 6 years ago

I agree with @matk86 that it is probably useful to have a convenience method to construct Lammpsdata from a structure/molecule. But I also agree with @adengz that the LAMMPS data file require more information. I suggest you just include a from_structure method, which will simply construct a topology from the sites and set up the cell with additional parameters.

I would suggest we do not allow construct from molecule. Instead, someone will have to go through get_boxed_structure to construct the simulation cell before passing it into Lammpsdata.

adengz commented 6 years ago

I will bring back a standalone method building LammpsData from structure soon, instead of a classmethod associated with LammpsData since this will ignore topologies and their force field parameters.

matk86 commented 6 years ago

Thanks. The use case I had in mind was not just simple boxed molecule but actual periodic structures with tilt etc computed from the given structures lattice and simple 'atomic' or molecule style (no need for topologies and forcefields, the reason why I had a separate lammpsforcefield class that inherited from lammpsdata)