jthorton commented 3 years ago

Description

This PR implements the ScanFilter and ScanEnumerator proposed in #59 by @SimonBoothroyd as a way to tag DOF for scanning. This also changes the default behaviour of the TorsiondriveDatasetFactory to not select every rotatable bond in a molecule for scanning. The factory will now only accept molecules that have torsions tagged for scanning and will remove any molecules not tagged. Users should now use the ScanEnumerator to select torsions by putting it in a workflow.

Notes

if the enumerator would match multiple times to a molecule only the first occurrence returned by openff.toolkit.topology.Molecule.chemical_environment_matches will be tagged for scanning.
If a molecule has multiple torsions tagged for scanning and matches the ScanFilter only the offending torsion will be removed meaning the molecule can still be put in a dataset if it has at least one valid torsion scan. ScanFilter will only remove the molecule from a dataset if the molecule has no torsions tagged after filtering.

Todos

Notable points that this PR has either accomplished or will accomplish.

[ ] TODO 1

Questions

[ ] Should we remove the ability to select torsions using the SmartsFilter and Coveragefilter to keep the logic in one place to select torsions (ScanEnumerator) ?

Status

[ ] Ready to go

lgtm-com[bot] commented 3 years ago

This pull request introduces 3 alerts when merging b0c85cde5e060f745483e985e212d1bec288bda6 into 3816027646ad5b725ad6601f02c6e396eb35d1b9 - view on LGTM.com

new alerts:

3 for Unused import

codecov[bot] commented 3 years ago

Codecov Report

Merging #149 (b08f781) into master (34de43b) will increase coverage by 0.42%. The diff coverage is 96.59%.

openforcefield / openff-qcsubmit

Torsion selection rework #149