Skip to content

Conversation

@mattwthompson
Copy link
Collaborator

This materializes my understanding of the internals into the README

If they do _not_ contain partial charges, the `openff-toolkit` charging method [`Molecule.compute_partial_charges_am1bcc`](https://open-forcefield-toolkit.readthedocs.io/en/latest/api/generated/openff.toolkit.topology.Molecule.html#openff.toolkit.topology.Molecule.compute_partial_charges_am1bcc) is used to assign partial charges.
The [canonical AM1-BCC charging method](https://docs.eyesopen.com/toolkits/cookbook/python/modeling/am1-bcc.html) is used to assign ELF10 charges if the OpenEye Toolkit is available.
If not, [Antechamber](http://ambermd.org/antechamber/) from the [AmberTools](http://ambermd.org/AmberTools.php) distribution (which uses the `sqm` semiempirical quantum chemical package) is used to assign AM1-BCC charges (`antechamber -c bcc`).
If the provided molecule(s) contain nonzero (user-specified) partial charges (stored in the `Molecule.partial_charges` attribute), those partial charges will be used.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To be frank, I would like to insert a comment to be explicit about how this behavior differs from SMIRNOFF (and the toolkit)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it could be helpful to add the note, better to be explicit about this behavior, I guess. On that note, do we want to support overriding this default behavior and forcing the toolkit to generate them if the user specifies it? (For future changes/PRs).

@mattwthompson mattwthompson marked this pull request as ready for review November 29, 2023 16:03
@mattwthompson
Copy link
Collaborator Author

I'm seeing this error:

FAILED openmmforcefields/tests/test_template_generators.py::TestSMIRNOFFTemplateGenerator::test_partial_charges_are_none - RuntimeError: C=O could not be fully assigned charges. Charges were assigned to atoms set() but the molecule contains {0, 1, 2, 3}.
FAILED openmmforcefields/tests/test_template_generators.py::TestSMIRNOFFTemplateGenerator::test_parameterize - RuntimeError: C=O could not be fully assigned charges. Charges were assigned to atoms set() but the molecule contains {0, 1, 2, 3}.

Which I've seen many times before but keep forgetting the source of

Copy link
Collaborator

@ijpulidos ijpulidos left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

If they do _not_ contain partial charges, the `openff-toolkit` charging method [`Molecule.compute_partial_charges_am1bcc`](https://open-forcefield-toolkit.readthedocs.io/en/latest/api/generated/openff.toolkit.topology.Molecule.html#openff.toolkit.topology.Molecule.compute_partial_charges_am1bcc) is used to assign partial charges.
The [canonical AM1-BCC charging method](https://docs.eyesopen.com/toolkits/cookbook/python/modeling/am1-bcc.html) is used to assign ELF10 charges if the OpenEye Toolkit is available.
If not, [Antechamber](http://ambermd.org/antechamber/) from the [AmberTools](http://ambermd.org/AmberTools.php) distribution (which uses the `sqm` semiempirical quantum chemical package) is used to assign AM1-BCC charges (`antechamber -c bcc`).
If the provided molecule(s) contain nonzero (user-specified) partial charges (stored in the `Molecule.partial_charges` attribute), those partial charges will be used.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it could be helpful to add the note, better to be explicit about this behavior, I guess. On that note, do we want to support overriding this default behavior and forcing the toolkit to generate them if the user specifies it? (For future changes/PRs).

@mikemhenry mikemhenry merged commit b5c14a1 into main Dec 20, 2023
@mikemhenry mikemhenry deleted the update-charge-docs branch December 20, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants