-
Notifications
You must be signed in to change notification settings - Fork 41
Membrane support #1561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Membrane support #1561
Changes from all commits
4e6617e
ac9862a
8ea145a
587a1bf
b7c63d6
24f3d58
617ac8c
7fd5ca9
18db828
1ee6fd7
238b36b
6945805
646ebbe
abf0007
96fa49a
b70e5b2
481ff35
4d28281
eff1958
1456a99
0cdde55
5bfd1c9
ce29130
c53fd60
5c313b1
9dce0e0
080d93e
48a7844
b492de0
2913994
e654a4d
1e707ba
bf51b0c
872c914
27d15f7
508fa62
a33d882
7a640ce
44e3545
6e4023d
339a9ed
741e44c
e53512b
e67ecd2
1f408fc
19761df
5d5bc8e
3e6b063
4d7da11
4c7cdd4
0962f3c
00c606e
b317cab
d97de53
2145f37
fc61675
b132806
88c9179
379cdf6
d79eb36
207fe3c
3504100
af99e37
c309957
9ae0565
4156c55
4327ef1
ff06372
3cd7793
8c00e32
5dd1758
11602f5
7ac6d32
45c26de
7dc626c
621b0d7
a42653e
825e8ae
919cacf
683b47d
21615c8
45f48e0
d3c4711
efdb5f1
5f6816c
b7f021d
cfdaf04
e35690c
5a9f873
31f9227
a2ab353
8ff8d45
39e62ca
71c8a99
edd0e5d
e23d2a2
1936b83
cafa04a
ec2beaa
a0a2ad1
6a3a5a3
0996c33
df381f9
54986e8
14e162e
a0b3c91
30218d2
5273d43
962dea5
c9b75b9
5eebbbc
8b53130
93b4cb7
481c0e9
32c4a5e
4586c5b
8b8529c
ede10d3
69d0986
40b87ec
ff2fec9
d7e4255
58a004d
0039bbe
5cfbc61
02ce210
3450d2c
78b266c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |||||||
|
|
||||||||
| import gufe | ||||||||
| from gufe import ( | ||||||||
| BaseSolventComponent, | ||||||||
| ChemicalSystem, | ||||||||
| ProteinComponent, | ||||||||
| SmallMoleculeComponent, | ||||||||
|
|
@@ -171,7 +172,8 @@ def _default_settings(cls): | |||||||
| ), | ||||||||
| solvent_solvation_settings=OpenMMSolvationSettings(), | ||||||||
| engine_settings=OpenMMEngineSettings(), | ||||||||
| integrator_settings=IntegratorSettings(), | ||||||||
| solvent_integrator_settings=IntegratorSettings(), | ||||||||
| complex_integrator_settings=IntegratorSettings(), | ||||||||
| restraint_settings=BoreschRestraintSettings(), | ||||||||
| solvent_equil_simulation_settings=MDSimulationSettings( | ||||||||
| equilibration_length_nvt=0.1 * offunit.nanosecond, | ||||||||
|
|
@@ -400,6 +402,11 @@ def _validate( | |||||||
| # Use the more complete system validation solvent checks | ||||||||
| system_validation.validate_solvent(stateA, nonbonded_method) | ||||||||
|
|
||||||||
| # Validate the barostat used in combination with the protein component | ||||||||
| system_validation.validate_protein_barostat( | ||||||||
| stateA, self.settings.complex_integrator_settings.barostat | ||||||||
| ) | ||||||||
|
|
||||||||
| # Validate solvation settings | ||||||||
| settings_validation.validate_openmm_solvation_settings( | ||||||||
| self.settings.solvent_solvation_settings | ||||||||
|
|
@@ -411,7 +418,11 @@ def _validate( | |||||||
| # Validate integrator things | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| settings_validation.validate_timestep( | ||||||||
| self.settings.forcefield_settings.hydrogen_mass, | ||||||||
| self.settings.integrator_settings.timestep, | ||||||||
| self.settings.complex_integrator_settings.timestep, | ||||||||
| ) | ||||||||
| settings_validation.validate_timestep( | ||||||||
| self.settings.forcefield_settings.hydrogen_mass, | ||||||||
| self.settings.solvent_integrator_settings.timestep, | ||||||||
| ) | ||||||||
|
|
||||||||
| def _create( | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| import openmm | ||
| import openmm.unit as omm_unit | ||
| from gufe import ( | ||
| BaseSolventComponent, | ||
| ChemicalSystem, | ||
| SmallMoleculeComponent, | ||
| settings, | ||
|
|
@@ -32,6 +33,7 @@ | |
| from openff.toolkit.topology import Molecule as OFFMolecule | ||
| from openff.units import Quantity, unit | ||
| from openff.units.openmm import from_openmm, to_openmm | ||
| from openmm import MonteCarloBarostat, MonteCarloMembraneBarostat | ||
|
|
||
| from openfe.protocols.openmm_md.plain_md_settings import ( | ||
| IntegratorSettings, | ||
|
|
@@ -180,14 +182,19 @@ def _create( | |
| # Validate protein component | ||
| system_validation.validate_protein(stateA) | ||
|
|
||
| # Validate the barostat used in combination with the protein component | ||
| system_validation.validate_protein_barostat( | ||
| stateA, self.settings.integrator_settings.barostat | ||
| ) | ||
|
|
||
| # Validate solvation settings | ||
| settings_validation.validate_openmm_solvation_settings(self.settings.solvation_settings) | ||
|
|
||
| # actually create and return Units | ||
| # TODO: Deal with multiple ProteinComponents | ||
| solvent_comp, protein_comp, small_mols = system_validation.get_components(stateA) | ||
|
|
||
| system_name = "Solvent MD" if solvent_comp is not None else "Vacuum MD" | ||
| system_name = "Solvent MD" if stateA.contains(BaseSolventComponent) else "Vacuum MD" | ||
|
|
||
| for comp in [protein_comp] + small_mols: | ||
| if comp is not None: | ||
|
|
@@ -363,9 +370,9 @@ def _run_MD( | |
| logger.info("Running NVT equilibration") | ||
|
|
||
| # Set barostat frequency to zero for NVT | ||
| for x in simulation.context.getSystem().getForces(): | ||
| if x.getName() == "MonteCarloBarostat": | ||
| x.setFrequency(0) | ||
| for force in simulation.context.getSystem().getForces(): | ||
| if isinstance(force, (MonteCarloBarostat, MonteCarloMembraneBarostat)): | ||
| force.setFrequency(0) | ||
|
|
||
| simulation.context.setVelocitiesToTemperature(to_openmm(temperature)) | ||
|
|
||
|
|
@@ -397,9 +404,9 @@ def _run_MD( | |
| simulation.context.setVelocitiesToTemperature(to_openmm(temperature)) | ||
|
|
||
| # Enable the barostat for NPT | ||
| for x in simulation.context.getSystem().getForces(): | ||
| if x.getName() == "MonteCarloBarostat": | ||
| x.setFrequency(barostat_frequency.m) | ||
| for force in simulation.context.getSystem().getForces(): | ||
| if isinstance(force, (MonteCarloBarostat, MonteCarloMembraneBarostat)): | ||
| force.setFrequency(barostat_frequency.m) | ||
|
|
||
| t0 = time.time() | ||
| simulation.step(equil_steps_npt) | ||
|
|
@@ -585,7 +592,7 @@ def run( | |
| solvent_comp, protein_comp, small_mols = system_validation.get_components(stateA) | ||
|
|
||
| # 1. Create stateA system | ||
| # Create a dictionary of OFFMol for each SMC for bookeeping | ||
| # Create a dictionary of OFFMol for each SMC for bookkeeping | ||
| smc_components: dict[SmallMoleculeComponent, OFFMolecule] | ||
|
|
||
| smc_components = {i: i.to_openff() for i in small_mols} | ||
|
|
@@ -634,6 +641,11 @@ def run( | |
| stateA_topology, | ||
| molecules=[s.to_openff() for s in small_mols], | ||
| ) | ||
| box = [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment explaining why this is necessary? |
||
| openmm.Vec3(*v.value_in_unit(omm_unit.nanometer)) | ||
| for v in stateA_system.getDefaultPeriodicBoxVectors() | ||
| ] * omm_unit.nanometer | ||
| stateA_topology.setPeriodicBoxVectors(box) | ||
|
|
||
| # f. Save pdb of entire system | ||
| if output_settings.preminimized_structure: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -470,9 +470,12 @@ def _check_and_store_system_forces(self): | |||||||||||||
|
|
||||||||||||||
| def _check_unknown_forces(forces, system_name): | ||||||||||||||
| # TODO: double check that CMMotionRemover is ok being here | ||||||||||||||
| known_forces = {'HarmonicBondForce', 'HarmonicAngleForce', | ||||||||||||||
| 'PeriodicTorsionForce', 'NonbondedForce', | ||||||||||||||
| 'MonteCarloBarostat', 'CMMotionRemover', 'CMAPTorsionForce'} | ||||||||||||||
| known_forces = { | ||||||||||||||
| 'HarmonicBondForce', 'HarmonicAngleForce', | ||||||||||||||
| 'PeriodicTorsionForce', 'NonbondedForce', | ||||||||||||||
| 'MonteCarloBarostat', 'CMMotionRemover', | ||||||||||||||
| 'CMAPTorsionForce', 'MonteCarloMembraneBarostat', | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| force_names = forces.keys() | ||||||||||||||
| unknown_forces = set(force_names) - set(known_forces) | ||||||||||||||
|
|
@@ -548,9 +551,11 @@ def _handle_box(self): | |||||||||||||
| """ | ||||||||||||||
| # Check that if there is a barostat in the old system, | ||||||||||||||
| # it is added to the hybrid system | ||||||||||||||
| if "MonteCarloBarostat" in self._old_system_forces.keys(): | ||||||||||||||
| known_barostats = ["MonteCarloBarostat", "MonteCarloMembraneBarostat"] | ||||||||||||||
| present_barostat = [i for i in self._old_system_forces.keys() if i in known_barostats] | ||||||||||||||
|
Comment on lines
+554
to
+555
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] this may be a bit cleaner
Suggested change
|
||||||||||||||
| if len(present_barostat) == 1: | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens otherwise? Throw an error? |
||||||||||||||
| barostat = copy.deepcopy( | ||||||||||||||
| self._old_system_forces["MonteCarloBarostat"]) | ||||||||||||||
| self._old_system_forces[present_barostat[0]]) | ||||||||||||||
| self._hybrid_system.addForce(barostat) | ||||||||||||||
|
|
||||||||||||||
| # Copy over the box vectors from the old system | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| ComponentMapping, | ||
| LigandAtomMapping, | ||
| ProteinComponent, | ||
| ProteinMembraneComponent, | ||
| SmallMoleculeComponent, | ||
| SolventComponent, | ||
| settings, | ||
|
|
@@ -199,6 +200,17 @@ def _adaptive_settings( | |
| if stateA.contains(ProteinComponent) and stateB.contains(ProteinComponent): | ||
| protocol_settings.solvation_settings.solvent_padding = 1 * offunit.nanometer | ||
|
|
||
| # adapt the barostat based on the system components | ||
| if stateA.contains(ProteinMembraneComponent) and stateB.contains(ProteinMembraneComponent): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It comes to mind now that you don't need to assert that both the states contain the thing, just one. |
||
| protocol_settings.integrator_settings.barostat = "MonteCarloMembraneBarostat" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an adpative settings entry for both ABFEs and SepTop? |
||
| protocol_settings.forcefield_settings.forcefields = [ | ||
| "amber/ff14SB.xml", | ||
| "amber/tip3p_standard.xml", | ||
| "amber/tip3p_HFE_multivalent.xml", | ||
| "amber/lipid17_merged.xml", | ||
| "amber/phosaa10.xml", | ||
| ] | ||
|
|
||
| return protocol_settings | ||
|
|
||
| @staticmethod | ||
|
|
@@ -539,6 +551,11 @@ def _validate( | |
| # Validate protein component | ||
| system_validation.validate_protein(stateA) | ||
|
|
||
| # Validate the barostat used in combination with the protein component | ||
| system_validation.validate_protein_barostat( | ||
| stateA, self.settings.integrator_settings.barostat | ||
| ) | ||
|
|
||
| # Validate charge difference | ||
| # Note: validation depends on the mapping & solvent component checks | ||
| if stateA.contains(SolventComponent): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,7 +169,11 @@ def _pre_equilibrate( | |
|
|
||
| # Don't do anything if we're doing a dry run | ||
| if dry: | ||
| return positions, system.getDefaultPeriodicBoxVectors() | ||
| box = [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? Why doesn't the ABFE unit need this fix too? (we use the |
||
| openmm.Vec3(*v.value_in_unit(omm_unit.nanometer)) | ||
| for v in system.getDefaultPeriodicBoxVectors() | ||
| ] * omm_unit.nanometer | ||
| return positions, box | ||
|
|
||
| # TODO: Refactor this part to live outside the method call | ||
| # We have to modify the output settings to have different output | ||
|
|
@@ -1374,4 +1378,11 @@ def run( | |
| **unit_result_dict, | ||
| } | ||
| else: | ||
| return {"debug": {"sampler": sampler}} | ||
| return { | ||
| "debug": { | ||
| "sampler": sampler, | ||
| "alchem_system": system, | ||
| "selection_indices": self.selection_indices, | ||
| "positions": equil_positions, | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of coherence later on, it would be better if this actually just returned the SolvatedPDBComponent.