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

Export positions with virtual sites by default #1078

Merged
merged 1 commit into from
Oct 15, 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
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()
Loading