ReactionMechanismGenerator / AutoTST

AutoTST: A framework to perform automated transition state theory calculations
Other
32 stars 16 forks source link

Fix of Conformer Analysis Issues for Migration TSs #85

Closed nateharms closed 3 years ago

nateharms commented 4 years ago

When I was digging into why AutoTST had a weirdly high failure rate for migration reactions, I dove into some of the results for the conformer analysis and saw conformers that look like the following:

For reaction rxn

The conformer generation outputted this as one of the low energy conformers where the stars are the reacting atoms:

Screen Shot 2020-06-16 at 1 38 39 PM

Diving into this I saw that one of the stared atoms is identified as a chiral center and is therefore inverted during the conformer generation process and this flips the geometry out of its ring-like TS. This PR will ignore chiral centers and CisTrans bonds in rings by looking at the _pseudo_geometry which has a fake single bond drawn between reacting atoms. An example of what the _pseudo_geometry looks like is below.

Screen Shot 2020-06-16 at 1 45 43 PM

codecov[bot] commented 4 years ago

Codecov Report

Merging #85 into master will increase coverage by 0.06%. The diff coverage is 78.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   65.18%   65.24%   +0.06%     
==========================================
  Files          27       27              
  Lines        4774     4791      +17     
==========================================
+ Hits         3112     3126      +14     
- Misses       1662     1665       +3     
Impacted Files Coverage Δ
autotst/conformer/systematic.py 75.11% <78.94%> (+0.61%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b90d4d9...10c602b. Read the comment docs.

nateharms commented 3 years ago

Hi, could someone take a look at this PR? @rwest @davidfarinajr @skrsna

nateharms commented 3 years ago

Sweet, I'm going to merge this in since @skrsna approved.