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

[WIP] Parameter coverage iPython notebook #419

Merged
merged 26 commits into from Sep 19, 2019
Merged

Conversation

j-wags
Copy link
Member

@j-wags j-wags commented Sep 12, 2019

Summary

This PR adds a parameter coverage checker hosted in an iPython notebook. This notebook produces a detailed report of which chemical groups a ForceField object is unable to parameterize. These chemistries are reported as

  • 2D molecule images, with unparameterizable groups highlighted (generated by RDKit)
  • tagged SMILES
  • tuples of atom indices

These results are printed out in the iPython notebook itself, and the final cells provide functionality to write to file as well.

My hope is that the Open Force Field industry partners can use this tool on a small dataset which encapsulates the diversity of the compounds they might simulate, and report critical areas where our force fields lack coverage. Industry partners probably can not reveal whole molecules from internal data sets, so I have tried to design this notebook such that it is easy to take screenshots of relevant chemistry, without revealing the whole molecule.

This notebook is suitable for analysis of hundreds to thousands of molecules, but probably not much more than that. It takes about 1 core-second per molecule, though I do offer a parallelized checker as well. Significant speedups past 0.1 sec/molecule would require several additional days of engineering, affect internal OFF Toolkit code, and decrease code readability. Thus, while we probably want to make a "fast" version of this some day, it would not be in the form of an interactive notebook.

Checklist

Status

  • Ready for review
  • Ready for merge

@lgtm-com
Copy link

lgtm-com bot commented Sep 19, 2019

This pull request introduces 1 alert and fixes 1 when merging a40dca1 into fe87bae - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Suspicious unused loop iteration variable

@codecov-io
Copy link

codecov-io commented Sep 19, 2019

Codecov Report

Merging #419 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #419   +/-   ##
=======================================
  Coverage   49.25%   49.25%           
=======================================
  Files          35       35           
  Lines        9577     9577           
=======================================
  Hits         4717     4717           
  Misses       4860     4860

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe87bae...a40dca1. Read the comment docs.

@j-wags j-wags closed this Sep 19, 2019
@j-wags j-wags reopened this Sep 19, 2019
@j-wags
Copy link
Member Author

j-wags commented Sep 19, 2019

Closed and reopened -- Trying to get travis to resume building this PR

@lgtm-com
Copy link

lgtm-com bot commented Sep 19, 2019

This pull request introduces 1 alert and fixes 1 when merging 501a1d8 into fe87bae - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 1 for Suspicious unused loop iteration variable

@j-wags j-wags merged commit bc89224 into master Sep 19, 2019
@j-wags j-wags deleted the parameter_coverage_example branch September 19, 2019 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants