Repository navigation
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
|
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
In this tutorial, we will demonstrate how to create a mixed...
Since the protocol also goes into the setup of the calculations, and analysis of results, I would add that here so the user has an overview of what's covered in the notebook.
Reply via ReviewNB
There was a problem hiding this comment.
Tried to do this with a minor edit to the overview and the list of steps.
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #4. ligands_file = "inputs/ligands.sdf"
This file still needs to be added. Do the ligand come with partial charges?
Reply via ReviewNB
There was a problem hiding this comment.
+1 a more descriptive file name would be great
There was a problem hiding this comment.
Added the file and updated the name.
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
I think the strategy for how to create the mixed network should be highlighted more.
Was there a specific reason you decided to go by series how and not by Lomap score?
Reply via ReviewNB
There was a problem hiding this comment.
Tried to make the selection method clear.
Was there a specific reason you decided to go by series how and not by Lomap score?
I was thinking that this is more consistent with our new domain of applicability guide and generally how we talk about the SepTop method and has no magic numbers (Lomap score cutoff) so this might be a nice intro to the idea of mixing the protocols.
In our other work though we do plan on having an API which can sub out the protocol based on lomap score so maybe we should change it here to plant the idea?
There was a problem hiding this comment.
I'm also not fully sure yet what the best way would be, yes, I agree we haven't investigated the science enough to say "lomap 0.5 is a great threshold", but I also wonder if planning the mixed network generally by score may be more something that would fit into the use cases of our users as if they already know there are different series and they should be treated separately, maybe they would just run them in completely separate maps. BUt if there was a score that could tell you generally which method is better (meaning also within a series), that would of course be better. So I think I would at least show a brief example on how to create the mixed network by score? Or do you think that would make the tutorial too confusing and that should be separate in e.g. a how-to?
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
openfe provides a number of free ene
[nit] Somehow "a number" sounds like a lot while we only have three binding protocols, I understand that you don't want to add a specific number as we will add more protocols, but maybe just "provides different free ..."? Not sure if that's better, please reject if you don't think so!
Reply via ReviewNB
There was a problem hiding this comment.
Tried to rephrase, is that a bit better?
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
[nit] probably worth adding the links for how partial charges can be added via the CLI & API.
Reply via ReviewNB
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #11. network_graph = nx.Graph()
Not specifically this line but all the lines in this cell - some comments that describe what you are doing would be useful to readers.
Reply via ReviewNB
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
[nit] How about moving the force field bit to a "warning" box that more specifically says that they need to ensure that key settings which affect the free energy results should match between the two Protocols - in this case the force field settings.
Reply via ReviewNB
| @@ -0,0 +1,1465 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
It seems to show on the github render I'll check on openfe.
No description provided.