search

openmm / openmm-cookbook

The OpenMM Cookbook and Tutorials
38 stars 10 forks source link

Not only querying, but also modifying system parameters #18

Closed yuanqing-wang closed 1 year ago

yuanqing-wang commented 1 year ago
yuanqing-wang commented 1 year ago

cross-ref: https://github.com/openmm/openmm-cookbook/issues/12

yuanqing-wang commented 1 year ago

@peastman how about I get rid of the separate "modify the parameters" one and simply append the existing "query the parameters" tutorial with ways to modify them in-place.

For the other one, I have seen so many people trying to using OpenMM to benchmark their own MM implementations. I feel like the community would benefit from a tutorial.

yuanqing-wang commented 1 year ago

see https://github.com/openmm/openmm-cookbook/pull/18/commits/38035e44cac7142561dfa1220288efbec358226b where I combined the modifying parameter tutorial with the querying parameter tutorial

yuanqing-wang commented 1 year ago

would love your feedback! @peastman @sef43

Perhaps the second item is more suitable for a tutorial than a cookbook?

sef43 commented 1 year ago

I think the setting different parameters is a good addition to the existing cookbook entry!

I think as you have suggested the notebook for "Use OpenMM to Verify Your MM Functional Form Implementation" should be a separate tutorial. Can you remove it from this PR and make a new PR adding that as a tutorial, we can then review it properly in the new PR.

yuanqing-wang commented 1 year ago

@sef43 I've made the changes as requested. Will follow up with another PR for the separate notebook.

yuanqing-wang commented 1 year ago

Separated PR here: https://github.com/openmm/openmm-cookbook/pull/22

peastman commented 1 year ago

I just realized it's missing one very important part. Let's add the following at the end.

Modifying Force objects will affect any new Simulations or Contexts you create, but it will have no effect on ones that already exist. If you want your modifications to apply to an existing Simulation, you can copy them over by calling bonded.updateParametersInContext(simulation.context).

yuanqing-wang commented 1 year ago

@peastman I've made the changes per your suggestions. I've also added the code to modify existing Context.

yuanqing-wang commented 1 year ago

I've updated the PR accordingly.

peastman commented 1 year ago

Just one unnecessary line that still needs to be removed. Then I think it will be ready to merge.

yuanqing-wang commented 1 year ago

Just one unnecessary line that still needs to be removed. Then I think it will be ready to merge.

Sorry which unnecessary line?

peastman commented 1 year ago 
bonded = [f for f in system.getForces() if isinstance(f, HarmonicBondForce)][0]
peastman commented 1 year ago

Looks good. Thanks!