openmm/openmmforcefields

Correct documentation -- partial charges

ijpulidos opened this issue · 3 comments

Currently the documentation for generating partial charges for molecules is not correct. For the current and modern openff-toolkit versions the method for assigning partial charges is no longer Molecule.compute_partial_charges_am1bcc but Molecule.assign_partial_charges. The documentation needs to be updated accordingly.

AFAICT

  • Charges in SMIRNOFFTemplateGenerator are not assigned here, but whatever is provided by the OpenFF stack. This is important because it means ELF10 charges are used if possible (the specified behavior)
    • Except in the case of input molecules already having partial charges, in which case they're forced through (notably a different behavior than the toolkit's current and historical defaults)
  • Charges in GAFFTemplateGenerator are assigned here, using the toolkit, and not attempting to use EL10 charges. (I'm not advocating it should.)

Something else I also noticed, If it matters: this line defaults to using OpenEye, if available, not RDKit. However, this line has no effect - unless conformer generation fails! - since the toolkit does not, by default, use the conformers attached to a molecule when assigning AM1-BCC charges.

@mattwthompson Thanks for this, and I agree with your comments, that does seem to be what we are doing with partial charges here, as far as I can tell.

I think we fixed this in #313