add draft for ff-output #442
Conversation
There was a problem hiding this comment.
Some comments and design remarks. I don't think sphinx will like your docstrings at the moment, but we can take a look at that later.
What I'm mainly missing is the why. Why do we want/need a FF writer?
As for the functionality of the code, I guess it looks ok, but the proof will be in the pudding. Ideally you'd make some tests where hypothesis creates a ForceField, you write it, you parse it, and you confirm they're the same. Undoubtedly there are corner cases missing here, such as [variables], and all the thinks links can do...
vermouth/ffoutput.py
Outdated
| self.normal_order_block_atoms = ["atype", "resid", "resname", | ||
| "atomname", "charge_group", "charge", "mass"] |
There was a problem hiding this comment.
Maybe these should be class attributes/variables rather than instance attributes/variables.
| for name, block in self.forcefield.blocks.items(): | ||
| self.stream.write("[ moleculetype ]\n") | ||
| excl = str(block.nrexcl) | ||
| self.stream.write(f"{name} {excl}\n") | ||
| self.write_atoms_block(block.nodes(data=True)) | ||
| self.write_interaction_dict(block.interactions) |
There was a problem hiding this comment.
Stupid suggestion: how do you feel about giving blocks a str/write/repr method instead?
Also for links/modifications/...
There was a problem hiding this comment.
We can move this one further up the chain, give it to ForceField. Note that I'm not convinced it's a good idea though, just an idea.
There was a problem hiding this comment.
Either way, group all these into a write_block function
vermouth/ffoutput.py
Outdated
| self.write_link_header(link) | ||
| self.write_atoms_link(link.nodes(data=True)) | ||
| self.write_interaction_dict(link.interactions) | ||
| self.write_edges(link.edges) |
| inter_dict: `class:dict[list[vermouth.molecule.Interaction]]` | ||
| the interaction dict to write | ||
| """ | ||
| for inter_type in inter_dict: |
There was a problem hiding this comment.
See also Molecule.sort_interactions
| def write_edges(self, edges): | ||
| """ | ||
| Writes edges to `self.stream` into the edges directive. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| edges: abc.iteratable | ||
| pair-wise iteratable edge list | ||
| """ | ||
| self.stream.write("[ edges ]\n") | ||
| for idx, jdx in edges: | ||
| self.stream.write(f"{idx} {jdx}\n") |
There was a problem hiding this comment.
Only write edges that are not already in interactions?
| Parameters | ||
| ---------- | ||
| edges: abc.iteratable | ||
| pair-wise iteratable edge list |
| self.stream.write("[ atoms ]\n") | ||
| for idx, (node, attrs) in enumerate(nodes): | ||
| idx += 1 | ||
| attr_line = " ".join([str(attrs[attr]) for attr in self.normal_order_block_atoms ]) |
There was a problem hiding this comment.
What about any other attributes?
| """ | ||
| self.stream.write("[ atoms ]\n") | ||
| for node_key, attributes in nodes: | ||
| attr_line = " {" + " ,".join([f"\"{key}\": \"{value}\"" for key, value in attributes.items()]) + "}" |
There was a problem hiding this comment.
The attr line is defined as json, so abuse that with json.dumps
| for interaction in inter_dict[inter_type]: | ||
| atom_string = " ".join(interaction.atoms) | ||
| param_string = " ".join(interaction.parameters) | ||
| meta_string = "{" + " ,".join([f"\"{key}\": \"{value}\"" for key, value in interaction.meta.items()]) + "}" |
|
@pckroon the why is easy: Because we already use that in 2 projects relating to automatic link generation from template itp-files. Also it is useful in the API as you can read in ff-files manipulate them not manually and then write the links again e.g. look for all links that do this and change attribute there. About the corner cases, they are not so bad. The input files are more condensed because of some hacks and tricks but once they are at link level everything gets resolved. The writer will not write the same force-field by line-comparison but a more verbose but also more robust version. For examples all variables should be resolved in the link, they just get written directly to the links. |
Ok, that sounds very reasonable. I personally also like being to both read and write things at the same time.
This I understand and agree with. But parsing the manual/condensed and the automatic/explicit FF must result in the same FF object. So comparison should be done on parsed FF level indeed. |
If the selector returns no coordinates the EN code break.
Skip EN generation for no-coordiantes
An initial draft for a ff-file writer. @pckroon some initial feedback would be appreciated. Once the general layout is good, I write tests.