Conversation
IAlibay
left a comment
IAlibay
left a comment
There was a problem hiding this comment.
Couple of small comments, but otherwise this looks great to me!
There was a problem hiding this comment.
looks great @hannahbaumann! Just a couple non-blocking edits and suggestions. I also opened #1954 so that the reference links work - but it can be merged independently from this PR.
| A thermodynamic cycle can be described as a set of :class:`.ChemicalSystem`\s (nodes) connected by | ||
| alchemical transformations (edges). The :class:`.Protocol` defines how the | ||
| :class:`.ChemicalSystem`\s map onto the cycle and how they are used in practice. |
There was a problem hiding this comment.
I think this unclear regarding thermodynamic cycles and alchemical networks. Not blocking for this PR, but maybe we can follow up with a better explainer over in the alchemical network docs?
There was a problem hiding this comment.
I ended up removing the reference to the alchemical networks here, because I think it would confuse users more, since a thermodynamic cycle is more linked to a Transformation. I think the reference to the protocols here is more helpful, since e.g. in the RBFE protocol we say how the ChemicalSystems are used to describe the thermodynamic cycle.
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
…FreeEnergy/openfe into user_guide_updates_membrane
| and disulfide bonds (defined via CONECT records). | ||
| * - :class:`.SmallMoleculeComponent` | ||
| - Ligands and cofactors | ||
| - |
There was a problem hiding this comment.
Do we want to say that they can optionally contain atomic partial charges which can be used in the simulation?
There was a problem hiding this comment.
Great idea, added this!
Co-authored-by: Josh Horton <Josh.Horton@newcastle.ac.uk>
Co-authored-by: Josh Horton <Josh.Horton@newcastle.ac.uk>
|
No API break detected ✅ |
jthorton
left a comment
jthorton
left a comment
There was a problem hiding this comment.
This looks great, good job @hannahbaumann!
Do we want to merge #1954 first though to make sure the links work in the docs or does it not matter?
4 participants
ProteinMembraneComponentexplanation, including example code of creating aChemicalSystemfor the complex leg of protein-membrane simulations._adaptive_settingsto the user guideChemicalSystems in ABFE and SepTop vs HybTop protocols.Checklist
newsentry, or the changes are not user-facing.pre-commit.ci autofix.Manual Tests: these are slow so don't need to be run every commit, only before merging and when relevant changes are made (generally at reviewer-discretion).
Developers certificate of origin