ReactionMechanismGenerator / ARC

ARC - Automatic Rate Calculator
https://reactionmechanismgenerator.github.io/ARC/index.html
MIT License
43 stars 21 forks source link

Fix H_Abstraction TS search heuristics for CH3 + RH <=> CH4 + R #595

Closed alongd closed 1 year ago

alongd commented 1 year ago

In the case of the reaction CH3 + RH <=> CH4 + R, we get that the ZMat constructed for CH4 has parameters which rely on the abstracted H atom. This is an edge case, usually higher angles and dihedral angles are not defined w.r.t H atoms where possible, but here there's no choice. The issue reported by @naddeu was that removing the H atom from the ZMat resulted in illegal ZMat parameters (a parameter that refers twice to a certain atom). This PR fixes this issue and adds a bunch of tests

fixes #638

codecov[bot] commented 1 year ago

Codecov Report

Merging #595 (dcf3c61) into main (fac0c4e) will increase coverage by 0.08%. The diff coverage is 99.11%.

@@            Coverage Diff             @@
##             main     #595      +/-   ##
==========================================
+ Coverage   73.04%   73.13%   +0.08%     
==========================================
  Files          99       99              
  Lines       26191    26270      +79     
  Branches     5484     5496      +12     
==========================================
+ Hits        19131    19212      +81     
+ Misses       5697     5696       -1     
+ Partials     1363     1362       -1     
Impacted Files Coverage Δ
arc/species/zmat.py 75.96% <97.43%> (+0.60%) :arrow_up:
arc/job/adapters/ts/heuristics_test.py 100.00% <100.00%> (ø)
arc/species/vectors.py 75.16% <100.00%> (+1.37%) :arrow_up:
arc/species/vectors_test.py 100.00% <100.00%> (ø)
arc/species/zmat_test.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

calvinp0 commented 1 year ago

@alongd Looks good to me

However, it appears to be failing on the zmat.remove_1st_atom w.r.t zmat_5.

E       -           'A_3_0_1': 63.02451429659178,
E       ?                              ^ ^ ^^^ ^
E       
E       +           'A_3_0_1': 63.024513203726876,

Looks like the calculated and expected are slightly off. However, when I pull this branch down and run the test, it passes fine... So I am not sure what is going on?

alongd commented 1 year ago

@kfir4444 and @calvinp0, are we all good to merge this?

calvinp0 commented 1 year ago

@kfir4444 and @calvinp0, are we all good to merge this?

I am happy with it

kfir4444 commented 1 year ago

@kfir4444 and @calvinp0, are we all good to merge this?

100%

alongd commented 1 year ago

Thanks!