Deprecate charge_from_molecules kwarg

[SimonBoothroyd]

Is your feature request related to a problem? Please describe.

It might be a good idea to begin thinking about deprecating the charge_from_molecules kwarg that is currently accepted by the ForceField->create_openmm_system function (and subsequently the ElectrostaticsHandler) as the LibraryChargeHandler is now able to more cleanly fill the same role.

This would begin to encourage users to store everything that is needed to parameterise a system within a force field, rather than splitting things between a force field and python scripts, and strengthen the idea the partial charges are a property of paricular force fields, rather than molecules.

Describe the solution you'd like

The deprecation should likely be accompanied by an API point for mapping a charged molecule into a new LibraryChargeType, e.g.

molecule = Molecule.from_XXX(...)
molecule.partial_charges = numpy.array(...) * unit.elementary_charge

charge_parameter = LibraryChargeType.from_molecule(molecule)

force_field = ForceField(...)
force_field["LibraryChargeHandler"].add_parameter(charge_parameter)

Describe alternatives you've considered

N / A

Additional context

N / A

[j-wags]

I fully agree with the implementation proposed by @SimonBoothroyd

[jchodera]

This feature was intended to enable rapid experimentation with new charge models without needing to implement the full machinery into the parameter handlers before assessing different models (e.g. the espaloma graph net charge model). Removing this feature would require us to fully implement every model we wanted to explore within the SMIRNOFF parameter assignment framework first---a large infrastructure investment of effort---just to try different ideas, would it not?

Is there any pressing reason to deprecate this? For true force field releases, we obviously don't use this feature. But that's not what it's intended for.

[SimonBoothroyd]

I'm not 100% sure I follow. You will still be able to provide custom charges without adding a custom handler, you will just need to add them as library charges rather than passing them via charge_from_molecules which would be made trivial by the API extension proposed above. This is just an API change that should remove redundant and simplify internal code (i.e. the kwarg and library charge handler do virtually the same thing), not a reduction of flexibility for users.

Are there use cases where providing charges as a library charge type would not work for you?

[jchodera]

I'm still not quite sure how this works in general. The example presumes that a LibraryChargeHandler is always present as part of the force field. What if it is not?

[davidlmobley]

I'm in favor of this if we can easily make a generalized LibraryChargeHandler but I'm not sure we have that functionality. On the other hand it may be trivial by a mechanism I haven't thought of yet. If it is, I support this.

We were just discussing earlier coming up with initial parameters for boron- and silicon-containing molecules and users may need to provide charges for these from molecules, at least initially, so we need to have some way of bringing in charges from molecules - even if it is via a LibraryChargeHandler that works as @SimonBoothroyd showed.

[mattwthompson]

The functionality exists now - any molecule's partial charges can be converted into a hyper-specific LibraryChargeType - but going back and doing the SMARTS matches is slow (#971), prohibitively slow for medium-ish and larger molecules. We'd need to fix that before considering a deprecation or removal of charge_from_molecules