Closed sef43 closed 1 year ago
Thanks! This looks like a good start. Here are some thoughts on improvements.
Could you add a paragraph at the beginning describing what umbrella sampling is and what it's used for? This is a tutorial, so we don't want to assume the reader already understands all the concepts. This is to teach them.
For the same reason, it's best not to assume they know what abbreviations like PMF, SMD, and WHAM stand for. The first time one of them comes up, write it out in full. For example, "computing a potential of mean force (PMF)."
Can you cite a publication for WHAM? They may want to learn about how it works.
You first create a CustomBondForce with only a single bond, then wrap it in a CustomCVForce to compute a function of that bond length. Why not combine them and compute the function directly in the CustomBondForce?
pullingForce = mm.CustomBondForce('0.5 * fc_pull * (r-r0)^2')
It will run faster, and the code will be simpler.
Does running the windows in parallel actually make much difference? Running multiple jobs at the same time on the same GPU generally doesn't work very well. It might help a bit on this system since it's so tiny, but on larger systems it's as likely to produce a slowdown as a speedup. It also risks running out of GPU memory or other resources, and it will fail if the device is set to exclusive mode.
This tutorial has a lot of run-on sentences, could you break them up, it will make the text more readable? :)
Thanks for the feedback! Agree with you about all the points about the text and explanations, will improve those.
You first create a CustomBondForce with only a single bond, then wrap it in a CustomCVForce to compute a function of that bond length. Why not combine them and compute the function directly in the CustomBondForce?
pullingForce = mm.CustomBondForce('0.5 * fc_pull * (r-r0)^2')
It will run faster, and the code will be simpler.
I agree for this example that is simpler and more efficient, however, I did it that way to make this tutorial easier to generalise. It shows how you can define a collective variable (which could be some other more complex function), and then how you can add a biasing potential to that collective variable. I think it makes it easier for someone to take this example and use a different collective variable. Also is there a way to report the current value of "r"
from the customBondForce
?
Does running the windows in parallel actually make much difference? Running multiple jobs at the same time on the same GPU generally doesn't work very well. It might help a bit on this system since it's so tiny, but on larger systems it's as likely to produce a slowdown as a speedup. It also risks running out of GPU memory or other resources, and it will fail if the device is set to exclusive mode.
For this tiny example on my Macbook (platform is OpenCL on the M2 gpu i believe) it does speed up the total simulation time, but yes this is not general best practice, I will change this. Although (off topic for this PR) in the world of big GPUs, multicore CPUs, and Cuda-MPS multi-replica simulations can definitely be more efficient when multiple process share GPUs, see this for GROMACS https://developer.nvidia.com/blog/maximizing-gromacs-throughput-with-multiple-simulations-per-gpu-using-mps-and-mig/ I wonder if OpenMM has similar behaviour?
There are some numbers for running OpenMM with MPS at https://github.com/openmm/openmm/issues/3082. It gives some speedup when running two simulations at once, especially for small systems. But you still wouldn't want to run 24 simulations at once.
This is ready for review again @peastman . I have added a lot more explanation
Is this ready for another review?
Is this ready for another review?
Yes it is thanks!
I have addressed the comments
Looks great. Go ahead and merge if it's ready.
Neither this nor the one from #26 is showing up on the docs page. Do we just need to wait longer? It's been a few hours since they were merged.
They are on the https://openmm.github.io/openmm-cookbook/dev/ page: https://openmm.github.io/openmm-cookbook/dev/notebooks/tutorials/umbrella_sampling.html https://openmm.github.io/openmm-cookbook/dev/notebooks/cookbook/Computing%20Interaction%20Energies.html
They wont be on the https://openmm.github.io/openmm-cookbook/latest page untill we manually cut a release on the github repo
and the main link https://openmm.github.io/openmm-cookbook is setup to redirect to https://openmm.github.io/openmm-cookbook/latest
Ok. Should we make a release now?
Yep I have done!
This PR adds a umbrella sampling tutorial we produced. It uses a small test system and covers the entire workflow of steered MD setup, running umbrella windows, computing a PMF with WHAM. We (@jmichel80) thought it would be good for inclusion in the OpenMM tutorials. Not sure how much overlap it has with #21