Skip to content

assign_partial_charges does not pass the toolkit_registry argument to openff-nagl #2149

Description

@lilyminium

Specifying the toolkit_registry argument to assign_partial_charges does not follow through to NAGL, meaning that (e.g. as below) you may wind up using OpenEye when you don't expect it.

Edit: turns out this isn't supported in NAGL anyway -- see openforcefield/openff-nagl#241

======= Text from @IAlibay ====
Thanks, the global toolkit registry now works, i.e. if I wrap things with toolkit_registry_manager, which is the minimum need on our end.

Edit: just to be clear - everything from this point onwards is beyond the minimum need, more just a curiosity because I couldn't work out what toolkit_registry was doing when not None.

However, maybe more of a problem for standard users - I do notice that manually passing the toolkit registry does not work, at least not in the way I would expect it to:

i.e.

from rdkit import Chem
from openff.toolkit import Molecule, RDKitToolkitWrapper, AmberToolsToolkitWrapper
from openff.toolkit.utils.nagl_wrapper import NAGLToolkitWrapper
from openff.toolkit.utils.toolkit_registry import toolkit_registry_manager, ToolkitRegistry

nagl_rdkit = ToolkitRegistry([RDKitToolkitWrapper(), NAGLToolkitWrapper()])

m.assign_partial_charges(
    partial_charge_method="openff-gnn-am1bcc-0.1.0-rc.1.pt",
    toolkit_registry=nagl_rdkit,
)

or different variants like

from rdkit import Chem
from openff.toolkit import Molecule, RDKitToolkitWrapper, AmberToolsToolkitWrapper
from openff.toolkit.utils.nagl_wrapper import NAGLToolkitWrapper
from openff.nagl.toolkits import NAGLRDKitToolkitWrapper, NAGLOpenEyeToolkitWrapper, NAGLToolkitRegistry
from openff.toolkit.utils.toolkit_registry import toolkit_registry_manager, ToolkitRegistry

nagl_rdkit = ToolkitRegistry([RDKitToolkitWrapper(), NAGLRDKitToolkitWrapper()])

m.assign_partial_charges(
    partial_charge_method="openff-gnn-am1bcc-0.1.0-rc.1.pt",
    toolkit_registry=nagl_rdkit,
)

or variants directly using NAGLRDKitToolkitWrapper don't seem to work.

It's not immediately clear to me that this should work, but there's code in this PR that makes me wonder if the intent is there but it's not doing the right thing (see my previous comment)?

Originally posted by @IAlibay in openforcefield/openff-nagl#237 (comment)

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions