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

Update lone pairs and multiplicities for radicals saturated for HBI #219

Conversation

pierrelb
Copy link
Contributor

Your multiplicity is taken from the original radical molecule and the hydrogens added to saturate the radical take the default number of lone pairs (-100). Updating them corrects the InvalidAdjacencyListError due to inconsistencies such as:

multiplicity 3
1  C U0 L0 E0  {2,S} {3,S} {5,S} {11,S}
2  C U0 L0 E0  {1,S} {4,S} {7,S} {12,S}
3  C U0 L0 E0  {1,S} {4,S} {8,S} {13,S}
4  C U0 L0 E0  {2,S} {3,S} {14,S} {15,S}
5  C U0 L0 E0  {1,S} {16,S} {17,S} {18,S}
6  C U0 L0 E0  {9,S} {19,S} {20,S} {21,S}
7  C U0 L0 E0  {2,S} {9,D} {22,S}
8  C U0 L0 E0  {3,S} {10,D} {23,S}
9  C U0 L0 E0  {6,S} {7,D} {24,S}
10 C U0 L0 E0  {8,D} {25,S} {26,S}
11 H U0 L0 E0  {1,S}
12 H U0 L0 E0  {2,S}
13 H U0 L0 E0  {3,S}
14 H U0 L0 E0  {4,S}
15 H U0 L0 E0  {4,S}
16 H U0 L0 E0  {5,S}
17 H U0 L0 E0  {5,S}
18 H U0 L-100 E0  {5,S}
19 H U0 L0 E0  {6,S}
20 H U0 L0 E0  {6,S}
21 H U0 L-100 E0  {6,S}
22 H U0 L0 E0  {7,S}
23 H U0 L0 E0  {8,S}
24 H U0 L0 E0  {9,S}
25 H U0 L0 E0  {10,S}
26 H U0 L0 E0  {10,S}

Are there other cases? Transport?
Thanks to @bslakman for the help

pierrelb added 2 commits May 27, 2014 12:44
The saturation of the radicals assigns the default value to
the lone pairs for the new hydrogen atoms (-100), and uses
the multiplicity for the radical molecule for the stable.

Without updating these attributes, the adjacency list
has an inconsistency and returns an error. This commit
should correct that issue.
Lone pairs and multiplicity taken from the radical, but
the saturation of the radicals should change these properties.
This should correct that issue.
@rwest
Copy link
Member

rwest commented May 28, 2014

This looks like an improvement to me so I'm merging, but I do wonder:

  1. how did Beat do it for the normal Benson thermo case?
  2. are we now doing very similar things in three or four places? Can we refactor (perhaps open an issue for now and fix it later)
  3. We probably do need it in Transport
  4. Is setting the multiplicity to radical count + 1 necessary and/or the best solution?

rwest added a commit that referenced this pull request May 28, 2014
Update lone pairs and multiplicities for radicals saturated for HBI
@rwest rwest merged commit 7c3c665 into ReactionMechanismGenerator:new-style-adjacency-list May 28, 2014
@pierrelb pierrelb deleted the new-style-adjacency-list branch June 13, 2014 17:44
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.

2 participants