Skip to content
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

Add option to enforce PBC when retrieving coords #8

Merged
merged 1 commit into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions femto/md/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,12 @@ class HREMD(BaseModel):
description="The number of cycles to run before saving the current replica "
"states to DCD trajectory files. If ``None``, no trajectories will be saved.",
)
trajectory_enforce_pbc: bool = pydantic.Field(
False,
description="Whether to apply periodic boundary conditions when retrieving "
"coordinates for writing to trajectory files.",
)

checkpoint_interval: int | None = pydantic.Field(
None,
description="The number of cycles to run before saving the current replica "
Expand Down
12 changes: 10 additions & 2 deletions femto/md/hremd.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ def _propagate_replicas(
replica_idx_offset: int,
force_groups: set[int] | int,
max_retries: int,
enforce_pbc: bool = True,
):
"""Propagate all replica states forward in time.

Expand Down Expand Up @@ -233,7 +234,9 @@ def _propagate_replicas(
simulation.step(n_steps)

local_replica_coords = simulation.context.getState(
getPositions=True, getVelocities=True
getPositions=True,
getVelocities=True,
enforcePeriodicBox=enforce_pbc,
)
femto.md.utils.openmm.check_for_nans(local_replica_coords)
except openmm.OpenMMException:
Expand Down Expand Up @@ -622,7 +625,10 @@ def run_hremd(
)

if initial_coords is None:
coords = [simulation.context.getState(getPositions=True)] * n_replicas
coords_0 = simulation.context.getState(
getPositions=True, enforcePeriodicBox=config.trajectory_enforce_pbc
)
coords = [coords_0] * n_replicas
else:
coords = [initial_coords[i + replica_idx_offset] for i in range(n_replicas)]

Expand All @@ -641,6 +647,7 @@ def run_hremd(
replica_idx_offset,
force_groups,
config.max_step_retries,
config.trajectory_enforce_pbc,
)

if mpi_comm.rank == 0:
Expand Down Expand Up @@ -672,6 +679,7 @@ def run_hremd(
replica_idx_offset,
force_groups,
config.max_step_retries,
config.trajectory_enforce_pbc,
)
reduced_potentials = mpi_comm.reduce(reduced_potentials, MPI.SUM, 0)

Expand Down
13 changes: 12 additions & 1 deletion femto/md/simulate.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def simulate_state(
stages: list[femto.md.config.SimulationStage],
platform: femto.md.constants.OpenMMPlatform,
reporter: femto.md.reporting.openmm.OpenMMStateReporter | None = None,
enforce_pbc: bool = False,
) -> openmm.State:
"""Simulate a system following the specified ``stages``, at a given 'state' (i.e.
a set of context parameters, such as free energy lambda values)
Expand All @@ -92,6 +93,8 @@ def simulate_state(
platform: The accelerator to use.
reporter: The reporter to use to record system statistics such as volume and
energy.
enforce_pbc: Whether to enforce periodic boundary conditions when retrieving
the final coordinates.

Returns:
The final coordinates and box vectors.
Expand Down Expand Up @@ -144,7 +147,11 @@ def simulate_state(
f"{femto.md.utils.openmm.get_simulation_summary(simulation)}"
)
coords = simulation.context.getState(
getPositions=True, getVelocities=True, getForces=True, getEnergy=True
getPositions=True,
getVelocities=True,
getForces=True,
getEnergy=True,
enforcePeriodicBox=enforce_pbc,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you're not using the config object like you are above in hremd.py?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah this function doesn't have a solo config, but rather a list of configs for each step, so there wasn't really a single good place to put this in a config or without changing the overall func signature that i could see.

)

return coords
Expand All @@ -157,6 +164,7 @@ def simulate_states(
stages: list[femto.md.config.SimulationStage],
platform: femto.md.constants.OpenMMPlatform,
reporter: femto.md.reporting.openmm.OpenMMStateReporter | None = None,
enforce_pbc: bool = False,
) -> list[openmm.State]:
"""Simulate the system at each 'state' using ``simulate_state``.

Expand All @@ -171,6 +179,8 @@ def simulate_states(
platform: The accelerator to use.
reporter: The reporter to use to record system statistics such as volume and
energy.
enforce_pbc: Whether to enforce periodic boundary conditions when retrieving
the final coordinates.

Returns:
The final coordinates at each state.
Expand All @@ -194,6 +204,7 @@ def simulate_states(
stages,
platform,
reporter if state_idx == 0 else None,
enforce_pbc=enforce_pbc,
)

final_coords = femto.md.utils.mpi.reduce_dict(final_coords, mpi_comm)
Expand Down
Loading