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

Intra add #148

Closed
wants to merge 7 commits into from
Closed

Intra add #148

wants to merge 7 commits into from

Conversation

nyee
Copy link
Contributor

@nyee nyee commented Nov 17, 2016

This pull request fixes the duplicate problem between Intra_R_Add_Exocyclic and Intra_R_Add_Endocyclic and adds some groups for rings in the Intra_R_Add_exocyclic family.

nyee and others added 4 commits November 17, 2016 15:44
These groups prevent *2 and *3 from being on a ring together, which will stop unwanted duplicates between endocyclic and exocyclic families
This prevents a radical carbon located two carbons away from a phenyl side
group from adding to either the meta or para positions, whih should be too strained to occur
For some reason I need to add these linear groups or larger ring groups with the equivalent number of labelled atoms cannot react.
…ation

These assume that a 6-membered ring or smaller is formed.
@mention-bot
Copy link

@nyee, thanks for your PR! By analyzing the history of the files in this pull request, we identified @connie, @jwallen and @mliu49 to be potential reviewers.

@zjburas
Copy link
Contributor

zjburas commented Nov 19, 2016

The latest commit forbids Intra_R_Add_Endo from ever adding to the double bond of an aromatic ring, such as in the following reactions:

image

image

Intra_R_Add_Exo already picks up these reactions in the other resonance form, so I think it is redundant for Endo to find it as well.

The new forbidden group does not, however, prevent reactions like the following from happening, which I think was a concern for @nyee ::

image

@mliu49
Copy link
Contributor

mliu49 commented Nov 19, 2016

I want to make sure I understand. So in general, we want to allow reactions like the latter, where *2 and *3 are the only labeled atoms on the ring, to happen in Intra_R_Add_Endocyclic (because they should behave similarly to linear reactants, and there's usually no risk of duplication with Exocyclic). However, for aromatics, the same reaction would be duplicated in Exocyclic due to resonance, which is what the latest commit fixes.

@zjburas
Copy link
Contributor

zjburas commented Nov 19, 2016

That is my understanding, but @nyee will have to confirm.

@nyee
Copy link
Contributor Author

nyee commented Nov 20, 2016

Nothing in this patch addresses the problem of aromatics which is a specific case of this issue .

The last commit is actually forbidding endo reactions where the radical is on the benzylic position. This is because the atom labelling changes, such that *4 is on the ring in this case. In the other cases, where the radical is further from the ring, the atom labelling with always have *5 on the ring.

@zjburas
Copy link
Contributor

zjburas commented Nov 20, 2016

@nyee , @mliu49 is referring to the latest commit above: 8508636. See the explanation above.

@nyee
Copy link
Contributor Author

nyee commented Nov 21, 2016

Oh sorry, I didn't see the commit. Yes this seems correct to me now.

@mliu49
Copy link
Contributor

mliu49 commented Nov 21, 2016

I have some observations and questions after testing a few reactants:

  1. 4d1723d forbid radicals two carbons away from the ring from reacting with the meta or para positions. However, radicals one carbon away from the ring can still react (eg. benzyl radical can still add to meta and para sites). Do we want to forbid those as well?
  2. The reaction degeneracies seem to be wrong. I'm testing with KineticsFamily.generateReactions(), so no resonance is being considered.
    • Observations:
      • For benzyl radical (ignoring how feasible these reactions are for now)
        • ortho: degeneracy = 2
        • meta: degeneracy = 4
        • para: degeneracy = 2
      • For propylbenzene and pentylbenzene primary radicals
        • ortho: degeneracy = 2
        • meta: degeneracy = 4
        • para: degeneracy = 1
      • For pentylbenzene primary radical
        • ortho: degeneracy = 1
        • meta: degeneracy = 2
        • para: degeneracy = 1
      • For 3-(methyl/ethyl/propyl/etc.)-cyclohexene primary radicals
        • ortho: degeneracy = 2
        • meta: degeneracy = 2
    • Hypotheses:
      • For aromatics, degeneracies are decreasing with chain length because we don't have sufficiently long groups
      • For all species with the double bond on a ring, we are matching both the linear backbone groups and the cyclic backbone groups. Therefore, we get twice as many atom mappings, and twice the degeneracy. Put another way, we implicitly assume that the groups in logic OR nodes are mutually exclusive, at least for degeneracy calculations. In this case, that's not true because the cyclic groups are a subset of the linear groups.
  3. For the new cyclic groups you added in Intra_R_Add_Exocyclic, why do beta groups have long and short, but alpha groups do not? I think this is the reason the meta degeneracies for aromatics are twice the ortho degeneracies, which I think should be equal in theory.

@zjburas
Copy link
Contributor

zjburas commented Nov 21, 2016

@mliu49 , Regarding question 1, yes, we should certainly make forbidden groups preventing benzylic carbon from adding to the meta or para position. I didn't forbid them in the initial commit because I was only thinking about my specific system.

It is also probably reasonable to forbid radicals 3 or even 4 carbons away from the ring from adding to the para position (and maybe meta too). I suppose it would be best to examine the potential TS's for these additions to decide what exactly should be forbidden. I can work on this.

from adding to either the meta or para position, in Intra_R_Add_Exo
family. Also forbid a phenyl group from undergoing self-ring-closure.
@zjburas
Copy link
Contributor

zjburas commented Nov 30, 2016

The latest commit forbids carbon radicals in either the alpha or gamma position (in addition to the eta already banned) relative to a phenyl side group F

@zjburas
Copy link
Contributor

zjburas commented Nov 30, 2016

The latest commit forbids carbon radicals in either the alpha or gamma position (in addition to the beta already banned) relative to a phenyl side group from adding to the meta or para positions. Examining the possible TS's for these reactions, they all seem unlikely to happen. Even forbidding such reactions out to the delta carbon radical might be justified.

The forbidden groups could also probably be made for general. For example, the linear side chain need not consist entirely of carbons, nor do all of the atoms need to be close-shell. Making these forbidden groups more general might help systems with heteroatom containing cyclics (like oxygenates). I don't know what the usual philosophy is regarding how general to make forbidden groups, so I will await your advice.

The same commit also bans the phenyl group from self-ring-closing to form a 3-5 or 4-4 membered bicyclic.

@mliu49
Copy link
Contributor

mliu49 commented Sep 20, 2017

Will be replaced by new pull request soon.

@mliu49 mliu49 closed this Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants