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

Add support for loading partial charges from SDF tags using new standard #250

Closed
j-wags opened this issue Apr 9, 2019 · 4 comments · Fixed by #281
Closed

Add support for loading partial charges from SDF tags using new standard #250

j-wags opened this issue Apr 9, 2019 · 4 comments · Fixed by #281

Comments

@j-wags
Copy link
Member

j-wags commented Apr 9, 2019

The OE and RDK maintainers have agreed on a spec for storing partial charges in SDF. We should implement support for I/O of this data.

@j-wags
Copy link
Member Author

j-wags commented Apr 10, 2019

Test suggestions from John:

(1) including a set of charged SDF files in our test dataset
(2) ensuring openforcefield.topology.Molecule can write charged SDF files via either RDKit or OpenEye
(3) adding tests of round-trips in all relevant directions between charged SDF files, OpenEye, RDKit, and Molecule?

I think this is exactly how we should go about this.

@j-wags j-wags modified the milestones: 0.2.2, 0.2.3 Apr 10, 2019
@j-wags j-wags self-assigned this Apr 15, 2019
@j-wags
Copy link
Member Author

j-wags commented Apr 19, 2019

Some notes as I move towards implementing this:

We will assume that partial charges should be written to a > <atom.dprop.PartialCharge> block in SDF files. Units for these values are assumed to be simtk.unit.elementary_charge (I'll consider whether we should explicitly specify this in another SD field)

It will generally follow the spec laid out here: rdkit/rdkit#2297

@j-wags
Copy link
Member Author

j-wags commented Apr 19, 2019

Note: Keep a record of what we end up calling the partial charge field in the table in #135

@jchodera jchodera changed the title Support for loading charges from SDF Add support for loading partial charges from SDF tags using new standard Apr 21, 2019
@jchodera
Copy link
Member

We should be sure to document (and version) the standard SDF tagging scheme for charges clearly in a readily accessible location so that it will be clear to other cheminformatics packages what the standard actually requires.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants