Repository navigation
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
- [nit] maybe we can put the line about "Choose and Configure a Protocol" inside one of those info boxes?
- [nit] How about a table for the parameters list?
- Thinking about thw iderr range of settings, maybe this should be "settings which impact sampling / performance", and then we could have "settings which affect the termodynamic state" and "settings that affect the model".
Reply via ReviewNB
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
"..., you may also need to consider increasing the number of lambda windows see the next section for more information." This sentence probably needs breaking into two sentences.
"Consider increasing the simulation length after running the simulation when:" Might be good to mention here that they'd have to run a new simulation (i.e. we can't extend at this time).
"Increasing the simulation length is not always the best solution...." This sentence is vague and maybe a bit a confusing. Technicaly you can just run a very long simulation to improve overlap - it might be good to say something like "increasing the number of windows can sometimes be more useful in increasing overlap". I.e. a combination of both would help, but think about lambda windows first.
Reply via ReviewNB
There was a problem hiding this comment.
- "A large number of samples are..": Maybe briefly mention where the user could find this information.
- Do you have a citation for the kbT cutoff?
IAlibay
left a comment
There was a problem hiding this comment.
Is this quite simple format okay to start with?
I think there's a lot of things here that could be generalized. As a first pass this is ok, but in follow-up work, it might be good to make this more abstract and make it cover multiple protocols.
Are there any other settings we should add in this first pass?
I left a comment above. I think it would be good to cover the broad "themes" of settings we have. E.g. things that control your model, things that control the thermodynamics, things that control sampling/convergence.
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
- Would we want a separate cookbook for SepTop/ABFE settings or could it make sense to showcase some difference here (mostly thinking of the lambda schedule).
- Should we also highlight and explain adaptive_settings in this cookbook or should that be done elsewhere? (maybe wrt what happens to the user updated settings after calling adaptive_settings, meaning it wouldn't be changed back to default settings); maybe also mention them in the section that talks about net charge changes?
Reply via ReviewNB
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
Equilibration length and protocol repeats are mentioned here, but not discussed below. Would it make sense to also add some more details for those (when one would want to change them, what the default is,..)?
Reply via ReviewNB
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
Could it make sense to generally have a section/table somewhere that has all the simulations diagnostics listed and then the possible settings changes, as a summary/overview?
Reply via ReviewNB
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #14. new_total_sim_time = n_repeats * n_windows * (protocol.settings.simulation_settings.production_length + protocol.settings.simulation_settings.equilibration_length)
Since the times are per leg, maybe we could call it *_sim_time_per_leg?
Reply via ReviewNB
| @@ -0,0 +1,305 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #5. # The number of replicas must be increased to match the number of lambda windows Maybe change this to must since it would raise an error
Reply via ReviewNB
Docs in response to OpenFreeEnergy/openfe#2278 this cookbook just explores the common settings users might want to change in the RBFE protocol.
Questions