Closed bannanc closed 5 years ago
As a side note - I recently noticed that you can now label PRs the same as issues, so I added a WIP label and am using that instead of [WIP] in the title.
OK, this PR is getting giant, but I'm still not done debugging or adding tests for the new code. Some updated:
I discussed included this in an updated to David, but I decided to try more stocastic approach for removing unnecessary decorators, basically make the Giant Cluster SMIRKS patterns and then remove decorators from them for a set number of iterations, only keep these removals if the automatically generated set matches the specified clustering.
This lead to needing some tools which are in [openforcefield](), primarily ChemicalEnvironments
which allows for these changes to SMIRKS patterns. I've only updated that code to be chemper compatible. Once I have this working and openforcefield
supports both OE and RDK I'll just make it a dependency.
ChemicalEnvironments
a long time ago to make it more pythonic. Specifically, I'd like to make the names more pep8 compatible and use the more pythonic @property
and @setter
functions instead of the Getters and Setters.As noted in issue #33 the OE license is expired. I'll make a separate PR to add the new one tomorrow, but both tests pass locally and the RDK tests pass here.
OK, this PR has snow balled A LOT, but I think it is read to go. Included here are a lot of utility functions and the introduction of the Reducer class. I still have some work to do on this class, but I think we need to get this infrastructure merged. The new notebook Generating Reasonable SMIRKS is the best demonstration of how the Reducer
class works currently and also includes documentation of what needs to be explored next.
I'll put in a PR with a new OE license in a few minutes, then this will be ready to go.
@vtlim is going to help me with at least a little code review before I merge this.
Comments on the ipynb of Generating reasonable SMIRKS.
Description
Step 1
Step 2
cluster_list[0]
is a non-empty dictionary specifying the ..."cluster_list
is very non-intuitive here since it looks like cluster_list[0]
should be double like the way you specify in smirks_labels
(which is also a different order than you wrote in the description).Step 3
Step 4
Thanks @vtlim
For the record on this PR:
ClusterGraph
I plan on fixing both to be more user intuitive, but in its own PR since it will require updating those examples as well. I merged this, there are TODOs in the code and I have some experiments I want to run with the best "moves" in the SMIRKSifier
, but those can happen in smaller steps. I did incorporate all of Vickie's feedback in my notebook.
In this pull request I have a jupyter notebook to show some initial attempts at how to shrink the complex SMIRKS patterns currently created by
ClusterGraph
.This led to a few improvements and utility functions in the rest of the code:
__str__
functions to the molecule classesClusterGraph
which moves decorators that are common to the end of the atom string with an AND ";
"[#6AH2X3x0r0+0,#6AH1X3x0r0+0:1]-;!@[#1AH0X1x0r0+0]
you would get[#6H2,#6H1;AX3x0r0+0:1]-;!@[#1AH0X1x0r0+0]
.Below are the things I'm still working on
ClusterGraph
technology as it stands.SMIRKS