Unimol accessibility #167
Merged
Unimol accessibility #167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@nyee, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jwallen, @connie and @mjohnson541 to be potential reviewers. |
Without specifically specifying the lone pairs, we do not correctly make this sample molecule because we do not have hypervalent sulfur yet.
-Changed some R to R!H because they were u2 or u3 and therefore impossible to be H -Added explicit charges to CO so that makeSampleMolecule cretes it correctly
reordering of trees to pass accessibility tests
nyee
force-pushed
the
unimol_accessibility
branch
from
29d9a22 to
da91201
Compare
-reordering of trees to pass accessibility, -Remove groups Cd_rad_out and Cdpri_rad_out and all their children because their valence is overspecified when taking into account the bond to the backbone
nyee
force-pushed
the
unimol_accessibility
branch
from
da91201 to
201f661
Compare
rearrangement of tree order to meet test,
-general rearrangements to meet accessibility tests -combined two group branches of rough form R2=R1(R3)H that had separate branches for R2 and R3 -moved Cs_H_out_OOH/H to be under the Cs_H_out_OOH group rather than the Cs_H_out_H/NonDeO -Added T bonds to R2H, which is the sampleBackbone. Previously when merged with a *2 Ct.
-rearranged groups to pass accessibility tests, -Switched a few `Cd,Cs` to `Cs,Cd`, which functionally will not change RMG performance, but now passes our unit test, unfortunately our makeSampleAtom is not as robust as we would like -Remove atom from Cd_rad_out to prevent it from possibly being over-specified -Remove the Cd atom and change name of Cd_rad_out_C_H, Cd_rad_out_C_ND, Cd_rad_out_C_De to Cd_rad_out_H, Cd_rad_out_ND, and Cd_rad_out_De because they were overspecified -Replaced Cd_rad_out_C with Cd_rad_out_double because it was overspecified 1b2e040
-rearrangement of entries to meet accessibility tests, -eliminated radadd_cdsingle subtree -made radadd_intra_Ct viable by removing the extra Ct to allow connection to backbone
-rearrangement of entries to pass accessibility tests -combination of the radadd_intra_cdsingle and radadd_intra_cddouble branches into one branch -fixed radadd_intra and it’s sub-entries by adding in the *4 hydrogens -also removed a Ct from radadd_intra_Ct to allow connection to backbone
-rearrangement of entries to meet accessibility tests -combination of radadd_intra_cddouble and radadd_intra_cdsingle branches into one branch -fixed radadd_intra and it’s sub-entries -added *4 labels/appropriate hydrogens to match backbone properly -removed a Ct from radadd_intra_Ct to allow connection to backbone
-rearrangement of entries to pass accessibility tests -Changed R to R!H for *1 and *2 since they have double bonds on the root backbone.
-removal of depreciated group CdsJ-2 from the CJ OR statement -rearrangement of entries in tree to pass accessibility tests -Change all *1 C to Cs in intra_substituionCS_cycilzation. Based on original backbones (from several PRs prior), this seems to be the original intent of the family.Also simplified the *1`C` subtree to `Cs` and eliminated entries for Cds and Ct in that subtree. -Cs-RRR is overspecified because the way the subtree is set up, *1 C should only have 2 more bonds: It and all children were changed to have one less atom. Most groups just reduced, but a few completely deleted: Cs-(TwoDe)S, Cs-(TwoDe)C, Cs-HHS, and Cs-HHC, and one group added Cs-HH -In a similar vein, the *3 tree was also overspecified for many entries. Used a similar approach to delete or simplify entries
-rearrangement of entries to pass accessibility tests -*3 subtree was overspecified for many groups. I have removed many of these groups and reduced the rest. This applies to all groups below CdsJ in that subtree. -Add two groups XSR4J_SS_Cs, XSR4J_SS_Ss which further specify atomtypes on backbones to accomodate rules -change rule number 2 to use groups XSR3J_S;C-HHH;CsJ-HH. I had to go back to commit 5f2c3c1 to see what the original intent was, but I'm fairly sure it is now correct
-rearrangement of entries to meet accessibility tests -removal of the redundant CJ group in favor of the CdsJ group -*3 subtree was overspecified for many groups. I have removed many of these groups and reduced the rest. This applies to all groups below CdsJ in that subtree -S-RR *1 and *2 subtree is also overpecified for many groups, removed many of these groups the same way we treated the *3 subtree. -Changed the following rules with indicated indices. They should all point to the original intent, but require less groups to do so: 2. XSR3J_S;SsJ-3-Cs;S-HC --> XSR3J_S_Cs;SsJ;S-H 3. XSR3J_S;CsJ-3-CsHH;S-HC --> XSR3J_S_Cs;CsJ-HH;S-H 4. XSR3J_S;CsJ-3-SsHH;S-HSs --> XSR3J_S_Ss;CsJ-HH;S-H 5. XSR3J_S;SsJ-3-Cs;S-Cs(HHH)C --> XSR3J_S_Ss;SsJ;S-H 6. XSR3J_S;CsJ-3-SsHH;S-Cs(HHH)Ss --> XSR3J_S_Ss;CsJ-HH;S-Cs(HHH) 7. XSR3J_S;CsJ-3-SsHH;S-Ss(H)Ss --> XSR3J_S_Ss;CsJ-HH;S-Ss(H) 8. XSR5J_SSS;CsJ-CsCsH;S-Cs(NonDe)C --> XSR5J_S_CsRCs;CsJ-CsH;S-Cs(NonDe) 9. XSR6J_SSSS;CsJ-CsCsH;S-Cs(NonDe)C --> XSR6J_S_CsRRCs;CsJ-CsH;S-Cs(NonDe) -Added a few backbone groups to accomodate rule changes: XSR3J_S_Cs, XSR3J_S_Ss, XSR5J_SSS_CsRCs, XSR6J_SSSS_CsRRCs
rearrangement of entries to meet accessibility tests -Remove an R atom on all backbones which should be part of an end groups -Also remove same unlabeled R atom on most general end group S-RR as it is not a reacting group and not necessary. This maintains the requirement that the subgraph of backbones must be the same as the most general end node. -*3 subtree was overspecified for many groups. I have removed many of these groups and reduced the rest. This applies to all groups below CdsJ in that subtree. -Changed names in rules accordingly -To accommodate rule number 3, I have created a new group which further specifies atomtype along the backbone, as is the only reasonable interpreation of the author's intent.
nyee
force-pushed
the
unimol_accessibility
branch
from
201f661 to
225c107
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request fixes accessibility for most unimolecular families. The exception is the Intra_R_Add_Exocyclic and Intra_R_Add_Endocyclic, which will be taken care of in a later pull request.