Skip to content

Commit

Permalink
Merge pull request #1078 from openforcefield/fix-1074
Browse files Browse the repository at this point in the history
Export positions with virtual sites by default
  • Loading branch information
mattwthompson authored Oct 15, 2024
2 parents 69b8955 + c447f1c commit 672643a
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docs/using/edges.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ surprising inconsistencies in some API points between different OpenFF tools.

Currently, the `Interchange.topology` attribute is defined by the OpenFF Toolkit's `Topology` object, which is feature-rich in cheminformatics functionality but not designed for MM interoperability. Most importantly, that representation does not include virtual sites (because molecules do not have virtual sites/dummy atoms). As a result, functionality involving virtual sites must go through `Interchange` API points instead of `Interchange.topology`.

For example, `Interchange.topology.get_positions()` will never include positions of virtual sites. To get the positions of a system with virtual sites included, use `Interchange.get_positions(include_virtual_sites=True)`. The default behavior of `Interchange.positions` is also to return positions without virtual sites.
For example, `Interchange.topology.get_positions()` will never include positions of virtual sites. To get the positions of a system with virtual sites included, use `Interchange.get_positions()`. The default behavior of `Interchange.positions` is also to return positions without virtual sites, but this may change in the future.

## Quirks of `from_openmm`

Expand Down
4 changes: 2 additions & 2 deletions openff/interchange/components/interchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,13 @@ def minimize(
else:
raise NotImplementedError(f"Engine {engine} is not implemented.")

def get_positions(self, include_virtual_sites: bool = False) -> Quantity:
def get_positions(self, include_virtual_sites: bool = True) -> Quantity:
"""
Get the positions associated with this Interchange.
Parameters
----------
include_virtual_sites : bool, default=False
include_virtual_sites : bool, default=True
Include virtual sites in the returned positions.
Returns
Expand Down
4 changes: 2 additions & 2 deletions openff/interchange/interop/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

def _to_positions(
interchange: "Interchange",
include_virtual_sites: bool = False,
include_virtual_sites: bool = True,
) -> Quantity:
"""Generate an array of positions of all particles, optionally including virtual sites."""
"""Generate an array of positions of all particles, optionally excluding virtual sites."""
if interchange.positions is None:
raise MissingPositionsError(
f"Positions are required, found {interchange.positions=}.",
Expand Down
4 changes: 2 additions & 2 deletions openff/interchange/interop/openmm/_positions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

def to_openmm_positions(
interchange: "Interchange",
include_virtual_sites: bool = False,
include_virtual_sites: bool = True,
) -> "openmm.unit.Quantity":
"""Generate an array of positions of all particles, optionally including virtual sites."""
"""Generate an array of positions of all particles, optionally excluding virtual sites."""
return _to_positions(interchange, include_virtual_sites).to_openmm()

0 comments on commit 672643a

Please sign in to comment.