ReactionMechanismGenerator / AutoTST

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

Fix-ups to Vibrational Analysis #79

Closed nateharms closed 4 years ago

nateharms commented 4 years ago

After investigating failed AutoTST jobs, it was appearing that our vibrational analysis needed a touch of work. After looking through the code, I noticed a few issues:

1) We were incorrectly labeling bonds in the reaction center for migration reactions. 3) The pseudo rdkit molecule that we created for migration reactions was incorrect. In most cases, the 2 atom is our H being abstracted, but in migration reactions, the 3 atom is that H. 1) We were not checking that reacting atoms were close enough to each other to react. 2) We were not checking if vibrations in the reaction center were above a specific amount and vibrations in the shell were below a specific amount.

This PR addresses these issues by:

1) Fixing the identification of reacting bonds. 2) Correctly creates our pseudo rdkit geometry. 3) Checks that active bonds are within 1.25x their predicted amount (e.g. if d12=1.5Å from our estimates and if measured d12<=1.25x 1.5 Å then we say that the atoms are close enough to actually interact). 4) Check that the average bond length in the reaction center changes by at least 15% and that the average bond length in the shell changes by no more than 5%.

codecov[bot] commented 4 years ago

Codecov Report

Merging #79 into master will increase coverage by 0.58%. The diff coverage is 84.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   64.66%   65.24%   +0.58%     
==========================================
  Files          27       27              
  Lines        4641     4854     +213     
==========================================
+ Hits         3001     3167     +166     
- Misses       1640     1687      +47     
Impacted Files Coverage Δ
autotst/species.py 73.84% <ø> (+1.33%) :arrow_up:
autotst/calculator/vibrational_analysis.py 79.38% <61.53%> (-6.92%) :arrow_down:
autotst/reaction.py 80.11% <100.00%> (+0.45%) :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 fb37081...6c82190. Read the comment docs.

nateharms commented 4 years ago

Okay, I updated based off of @davidfarinajr's recommendations. Can you look over it again?

nateharms commented 4 years ago

Thanks for the catch on the typo! Just rebased and pushed, will merge in once it passes the checks. Then we can get yours merged in too!