ReactionMechanismGenerator / AutoTST

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

linear torsion angle fix #69

Closed davidfarinajr closed 4 years ago

davidfarinajr commented 4 years ago

I added a method within get_torsions that checks for linear (or near-linear within a tolerance) angles in a torsion. If an angle is near 180 degrees, we look farther down the molecule for an atom to define the torsion. I also added angles, description, and center_atoms attribute to torsion class.

davidfarinajr commented 4 years ago
Screen Shot 2020-03-16 at 9 04 04 PM

It's helpful for molecules like FCC#CC. Previously, the get_torsions method would return 2 torsions. With this PR, 1 torsion is returned where the terminal carbons are the pivot atoms.

codecov[bot] commented 4 years ago

Codecov Report

Merging #69 into master will increase coverage by 0.28%. The diff coverage is 90.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   63.78%   64.07%   +0.28%     
==========================================
  Files          27       27              
  Lines        4667     4721      +54     
==========================================
+ Hits         2977     3025      +48     
- Misses       1690     1696       +6
Impacted Files Coverage Δ
autotst/species_test.py 98.73% <100%> (+0.06%) :arrow_up:
autotst/geometry.py 89.13% <100%> (+0.49%) :arrow_up:
autotst/species.py 72.5% <89.39%> (+1.25%) :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 20b75b7...23744c2. Read the comment docs.

nateharms commented 4 years ago

Also, testing failed and coverage is down, these will need to be addressed before we can merge this in.

davidfarinajr commented 4 years ago
Screen Shot 2020-03-17 at 10 34 51 AM Screen Shot 2020-03-17 at 10 40 39 AM

This is rotor scan for SMILES FCC#CCF with C1 and C2 as the pivot atoms (F1-C1-C2-F2). Currently, AutoTST generates 2 torsions for this molecule: (F1-C1-C3-C4) and (F2-C2-C4-C3). However, both of these scans failed because C1-C3-C4 and C2-C4-C3 are linear. The goal of this PR is to identify rotors where the axis of rotation has more than 2 atoms (pivot atoms are not directly connected in graph).

The center_atoms are the atoms in between the pivot atoms (in this case C3 and C4).

davidfarinajr commented 4 years ago

@nateharms, I added center_atoms test. I also realized we need to consider the center_atoms when getting the mask, so I modified the get_mask method and refactored the get_torsions method a bit. Can you take a look at the new changes?

nateharms commented 4 years ago

Will do, I'll wait a sec for the checks to pass, then I'll review and merge if it's all good.