search

peteboyd / lammps_interface

automatic generation of LAMMPS input files for molecular dynamics simulations of MOFs
MIT License
125 stars 63 forks source link

Improve CIF parser to accept symmetric MOFs. #5

Open kbsezginel opened 6 years ago

kbsezginel commented 6 years ago

I think the CIF parser can be improved to accept non-P1 MOFs. ASE Python library has a pretty good CIF parser so that can be used to read MOFs or even other crystals. Moreover, it can read many file formats, allowing other file formats to be accepted. I would be willing to work on this.

tawe141 commented 4 years ago

Has there been any work on this? This would also fix #1

ltalirz commented 4 years ago

I don't think so - right @peteboyd ?

ltalirz commented 4 years ago

This is something that would be quite simple to add, in particular once we have the first basic continuous integration tests in place (which would guard against unintended consequences of the shift)

ltalirz commented 3 years ago

@peteboyd It turns out Henglu just stumbled upon this as well.

Would you be open to adding a dependency on ASE to get a fully fledged CIF parser?

I see that you already thought about this in https://github.com/peteboyd/lammps_interface/blob/9983f2953b231bb079a09361c2972eacb9ce10d3/lammps_interface/structure_data.py#L1579-L1589

Is there more to do than make ase_from_CIF return the MolecularGraph and the cell like from_CIF does? Does lammps-interface use aspects of the CIF file other than elements and positions of the atoms?

If it's just the above, I think it would be quite straightforward to do.

peteboyd commented 3 years ago

The only thing that would require a little attention is when the code reads in bonding information from the CIF file. I'm not sure if that part of the code is still relevant, but it would have to be investigated before proceeding with the ASE implementation.

SeyedMohamadMoosavi commented 3 years ago

Reading the bonding information from CIF is absolutely useful when we have structures generated using one of the topology-based structure generators. I suggest switching to ASE for the general cases and adding a new argument input when one needs to read the bonding topology from CIF (keeping the current pieces of the code). This has an extra advantage as ASE periodic distance computation is much faster than the one in lammps interface --> it allows dealing with larger MOFs.

peteboyd commented 3 years ago

Reading the bonding information from CIF is absolutely useful when we have structures generated using one of the topology-based structure generators. I suggest switching to ASE for the general cases and adding a new argument input when one needs to read the bonding topology from CIF (keeping the current pieces of the code). This has an extra advantage as ASE periodic distance computation is much faster than the one in lammps interface --> it allows dealing with larger MOFs.

Thanks - good point about the weird bonding cases that come from crystal building programs. Bonding can be dealt with using symmetry operations, we just have to be careful about it. I recall the notation in the CIF format is a bit strange.