Conversation
IAlibay
left a comment
There was a problem hiding this comment.
I think we've historically been missing some exposition here, so my question would be if it would be reasonable to add it or if it would end up being too much work (or if it should be elsewhere)?
| .. warning:: | ||
|
|
||
| The ``_adaptive_settings()`` method is experimental and subject to change. |
There was a problem hiding this comment.
This is great. As a small nit, it might be good to move that to the top so that it's the first thing folks read.
| 2. components can be reused to compose different systems. | ||
| 3. :class:`.Protocol`\s can have component-specific behavior. E.g. different force fields for each component. | ||
|
|
||
| When dealing with membrane-protein systems, the protein is represented using an explicitly solvated |
There was a problem hiding this comment.
The content of what is being added looks good, but there's a "missing link".
What I think we've been missing (since before membrane support) is the idea that the Components are composable. What would you think about something like this:
- These Components are additively added into a ChemicalSystem, which will define the chemical composition of the simulated system.
- For a conventional protein-ligand complex in water, this would look like [add protein, small and solvent components]
- Some mention that the protein component handles crystallographic waters and also can define SSBONDs via the CONECT records.
- For membrane proteins: what you have here
Co-authored-by: Irfan Alibay <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1933 +/- ##
==========================================
- Coverage 94.69% 90.65% -4.05%
==========================================
Files 210 210
Lines 18808 18808
==========================================
- Hits 17811 17050 -761
- Misses 997 1758 +761
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…FreeEnergy/openfe into user_guide_updates_membrane
IAlibay
left a comment
There was a problem hiding this comment.
Couple of small comments, but otherwise this looks great to me!
|
No API break detected ✅ |
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