A draft for read and write bond type for lammps data

[Roy-Kid]

Dear @Luthaf and other developers,

According to the previous discussion #476 , I write an untested draft that read and write bond type from lammps data file. In this PR, I want to read and write bond/angle/dihedral data from lammps data and trajectory.

Since issue #462 is planning to support symbolic atom type, I choose std::string as the representation for bond type.

In the following code, I completely complied with the storage method of BondOrder. But here I want to initiate a discussion about whether to use bond type or even bond order as a member attribute of Bond, like type in Atom. I think the benefits are as follows: The form is tidier, and the bond information can be obtained directly by iterating topology.bonds

I hope you can review my code in your free time so that I can complete these features in the future~

Thanks a lot!

[Luthaf]

Sorry for the very long delay for the review. This looks pretty good overall!

My main concern is about the handling of bond types in the LAMMPS code. It looks like currently the code will create a set containing all bonds, where we would like to only have one entry per bond type (which should be a much smaller number).

I think the code should try to look for bond types in the topology, and if it can not find one, fallback to the old bond type generation.

---

The new bond type parameter to add_bond should also be added to the C API, to make it accessible from Python/Rust/Julia/...

[Luthaf]

Suggested change

	std::string bond_type(size_t atom_i, size_t atom_j) const;
	const std::string& bond_type(size_t atom_i, size_t atom_j) const;

this should return a const reference instead of a copy

[Roy-Kid]

@Luthaf Hi, sorry for bothering you.
I re-wrote the code according to your suggestion. Now if it not find a custom bond type, it will degrade to integer bond type.
However, the custom bond type to integer bond type is one-to-one mapping, but the add_bond API is per-bond type mapping. For example, if 1-2 and 1-3 both bond type 1, after I do add_bond(1, 2, order, "typeA") and add_bond(1, 3, order, "typeB"), the bond type in lammpsdata is both typeB. The later bond_type will override the former one.
Should we use the rule: check if user defines a per-bond type, for example, add_bond(1, 2, order, "typeA"). If so, use "typeA" instead of the original type 1, if not, degrade it to type 1;

[Luthaf]

However, the custom bond type to integer bond type is one-to-one mapping, but the add_bond API is per-bond type mapping. For example, if 1-2 and 1-3 both bond type 1, after I do add_bond(1, 2, order, "typeA") and add_bond(1, 3, order, "typeB"), the bond type in lammpsdata is both typeB.

I'm not sure I understand here. add_bond is the chemfiles function, right? In this case, why would add_bond(1, 2, order, "typeA") followed by add_bond(1, 3, order, "typeB") result in both bonds having typeB? I would expect that this results in two bonds (1-2 with typeA; and 1-3 with typeB). I'll re-read through the code as well to try to understand.