michellab / BioSimSpace

Code and resources for the EPSRC BioSimSpace project.
https://biosimspace.org
GNU General Public License v3.0
77 stars 19 forks source link

[TUTORIAL] Steered MD #192

Open lohedges opened 3 years ago

lohedges commented 3 years ago

This is a thread to discuss the creation of a tutorial showing how to implement steered molecular dynamics in BioSimSpace.

lohedges commented 3 years ago

Okay, moving it later gives a different error. I'll need to think about this since it looks like the indexing for the evaluator might not work properly depending on the number of coordinate groups in the different molecules. If you could just use your pre-computed value from MDTraj for now and see if the streered MD PLUMED file works, that would be great.

(I tested this RMSD evaluation on about 10 moleclue pairs on Friday, bah!)

AdeleHardie commented 3 years ago

Alright, I've just commented out the lines in __init__ and I'll proceed with a precomputed RMSD value. Otherwise the units are working well.

Another thing that I noticed is that when I create a GROMACS process, the arguments do not include PLUMED:

>type(protocol)
BioSimSpace.Protocol._steering.Steering
> process = BSS.Process.Gromacs(system, protocol)
> process.getArgs()
OrderedDict([('mdrun', True), ('-v', True), ('-deffnm', 'gromacs')])

I believe it should be:

OrderedDict([('mdrun', True),
             ('-v', True),
             ('-deffnm', 'gromacs'),
             ('-plumed', 'plumed.dat')])

as is for Metadynamics

lohedges commented 3 years ago

Thanks for catching, I'll update now.

I've tested the RMSD evaluator and it works if I do it on a per atom basis, rather than passing the entire dictionary of matches in one go. I'm still not sure why that's not working, since it does work for other molecules and for subsets of the matches with this pair. I've checked that it agrees with the distance computed in the space, so I'll just do it per atom and take the average.

lohedges commented 3 years ago

I've got something working just looping over each matching atom pair and computing the RMSD myself using the system space property. For the initial value of the CV I get 31.4116 A, which doesn't seem to match the value in your example file. What do you get with MDTraj for this system? (The example PLUMED file has 0.55 nm as the starting value.)

AdeleHardie commented 3 years ago

the example value I think may be a "close enough" value. I get 0.32 nm with PLUMED, so I think that's close enough. Depending on alignment algorithms RMSD values can slightly differ I think.

lohedges commented 3 years ago

Whoops, I didn't realise that it had performed an RMSD alignment using the alignment indices prior to computing the RMSD. If that's the case, then I get the same value:

cv.getInitialValue().nanometers()
0.3236 nm
lohedges commented 3 years ago

The updated CV calculation has now been pushed.

lohedges commented 3 years ago

How's it working. I'll merge across to devel if things look okay and continue working on improving the restraint handling.

I just had a quick look through your docs and it all looks good. One thing that I hadn't realised is that it uses the system from this issue, i.e. with the strange unfolding issue despite single-point energies agreeing with ParmEd output, etc. It would be good to revisit this before the workshop since I don't think it's a good look to start a tutorial with input that we know is problematic, at least without being able to provide some kind of reason for the discrepancy. I chatted with @chryswoods about this a while ago (since he wrote the AmberPrm parser) and he suggested looking at snapshots from your trajectory and comparing single point energies along it. If we agree, then we must be sampling the same energy surface.

AdeleHardie commented 3 years ago

I've ran some short tests and it's all working great, I'm also hoping to do full-length benchmarking on the cluster.

I'm sorry I didn't make it more clear it's the same system! I thought I'd mentioned it at one of the initial meetings. What would be a good way to share the data with you? The trajectory file would be too big, but I could pull a few frames from it?

lohedges commented 3 years ago

Fantastic, glad to hear that it's working. No worries, I think it's good to show how BioSimSpace is being used in an actual and active research project, rather than just using canonical examples that aren't that interesting (any more) and are known to work.

Yes, just pull some frames at regular intervals. I imagine 10 should be enough to see if there's a problem. We can choose to get more fine grained if it turns out that something goes wrong after some period of time.

lohedges commented 3 years ago

I was also thinking, if AMBER supports steering in exactly the same way as GROMACS than I could just expose the PLUMED options to AMBER too. If it's the same for metadynamics too, then I could also add this in. I guess the issue is that it only works with pmemd, right? If so, then I'd need to check the executable, which might be tricky if it has a non-standard name. (Hopefully pmemd reports something more interesting than sander if it's run with no input.`)

lohedges commented 3 years ago

Okay, I've merged across into devel and will make further fixes there.

AdeleHardie commented 3 years ago

PLUMED works with sander as well, the config files are the same (at least from what I remember from when I started working with PLUMED and tested sander).

lohedges commented 3 years ago

Oh great, that makes things very easy then. I'm not sure why we only supported GROMACS by default then. I guess the metadynamics folks we were working with only used GROMACS and I was led to believe that there was something more complicated with AMBER.

lohedges commented 3 years ago

Direct metadynamics and steered metadynamics support with AMBER is now available in devel. Let me know if it works for you.

lohedges commented 3 years ago

Just a heads up that you can use process.getInput() to get a zip file containing all of the input files needed for simulation. This would save the manual zip stage in your setup notebook.

AdeleHardie commented 3 years ago

That's a good point! The manual zip is a relic from before native Amber sMD, I will change it.

lohedges commented 3 years ago

For working out the indices of the atoms in the RMSD residues you could use the built in search functionality. Not sure it's too much clearer than what you have, but avoids the need of a string comparison that a user might not know how to do without digging into the code a little deeper. For example:

# Here a list of the residues of interest as strings so we can use them in our search.
resnums = [str(x) for x in range(174, 185)]

# Perform the search.
atoms = system.search(f"atoms in resnum {','.join(resnums)} and not element H")

# Now work out the absolute indices of the atoms in the system.
idxs = []
for atom in atoms:
    idxs.append(system.getIndex(atom))
lohedges commented 3 years ago

I've updated the code so that rmsd_indices is now a required parameter. I also check whether rmsd_indices matches all atoms in the reference and raise an exception if so, i.e. no atoms would be used for alignment. Since rmsd_indices is now mandatory you would need to re-order your arguments when creating the rmsd_cv, e.g.:

rmsd_cv = BSS.Metadynamics.CollectiveVariable.RMSD(system, reference, rmsd_indices, 0)

This won't be an issue for the workshop runs, since they'll be using the old build, but we'll want to update things for the paper. (The paper will hopefully coincide with a new release of BioSimSpace so that people can use the exact same version when following it.)

AdeleHardie commented 2 years ago

I'm reviving this discussion since it's a continuation of the work that was done before to implement sMD into BSS (feel free to move it to a new issue). As I have been using BSS to run a variety of sMD in the past year, I have some small fixes that I now made on feature-smd branch. They mainly concern using RMSD for more complicated use cases:

  1. 349791c writing multiple RMSD references
  2. 970417c correctly number RMSD reference
  3. ffa926a default fixing of verse in MOVINGRESTRAINT (this is not neccessary but it tripped me up when I was doing things so I think it would be nice to include)

Let me know if you are happy with them, or if you want to redo them in a way that is more consistent with how BSS is written.

lohedges commented 2 years ago

Thanks, @AdeleHardie. Feel free to open a pull request when you are happy with things and I can review. At a quick glance everything looks fine to me. Are you planning on adding any further edits, or is this it for the time being?

Cheers.

AdeleHardie commented 2 years ago

I would also like to add an additional CV for evaluating a custom expression from other CVs (PLUMED documentaiton). I was going to do a feature request issue once I finish with the example code. This is a bigger change and isn't necessary to have a working tutorial for the BSS workshop. Would you prefer for me to just open a pull request for these simple changes for now, and set up a separate branch/issue for the new CV?

lohedges commented 2 years ago

Yes, that makes sense. The fixes relate to existing functionality whereas the additional CV will be a new feature, so would be best implemented on a new branch with separate PR.

Cheers.