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

Output RMG libraries in Arkane #1769

Merged
merged 4 commits into from
Oct 30, 2019
Merged

Output RMG libraries in Arkane #1769

merged 4 commits into from
Oct 30, 2019

Conversation

alongd
Copy link
Member

@alongd alongd commented Oct 17, 2019

Motivation or Problem

The current Arkane output is not in a format that can be directly pasted into RMG libraries. See #1768

Description of Changes

Now Arkane will output thermo and kinetic libraries into an RMG_libraries folder. Note that only species with structure will be considered for the thermo library, and only reactions with structures for all their reactants and products will be considered for the kinetics libarry.

Testing

Added a unit test for an intermediate newly added function, could add functional tests as well.

Reviewer Tips

Run a thermo job, specifying structure for a species, and see the resulting thermo library.
Run kinetics and pdep jobs, specifying structures for all species, and see the resulting kinetics library.

@codecov
Copy link

codecov bot commented Oct 17, 2019

Codecov Report

Merging #1769 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1769   +/-   ##
=======================================
  Coverage   32.61%   32.61%           
=======================================
  Files          87       87           
  Lines       26124    26124           
  Branches     6878     6878           
=======================================
  Hits         8521     8521           
+ Misses      16644    16633   -11     
- Partials      959      970   +11
Impacted Files Coverage Δ
rmgpy/data/statmech.py 42.2% <0%> (ø) ⬆️
rmgpy/rmg/pdep.py 12.21% <0%> (ø) ⬆️
rmgpy/reaction.py 0% <0%> (ø) ⬆️
rmgpy/data/kinetics/family.py 48.35% <0%> (ø) ⬆️
rmgpy/yml.py 15.71% <0%> (ø) ⬆️
rmgpy/data/kinetics/database.py 50.61% <0%> (ø) ⬆️
rmgpy/rmg/input.py 34% <0%> (ø) ⬆️
rmgpy/molecule/molecule.py 0% <0%> (ø) ⬆️
rmgpy/statmech/ndTorsions.py 59.78% <0%> (ø) ⬆️

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 be1de4b...aa745ef. Read the comment docs.

Copy link
Contributor

@mjohnson541 mjohnson541 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, one question about the last commit.

@@ -1103,6 +1103,7 @@ def assign_frequency_scale_factor(freq_level):
'ccsd(t)-f12/aug-cc-pvtz': 0.998, # [3], taken as CCSD(T)-F12a/cc-pVTZ-F12
'ccsd(t)-f12/aug-cc-pvqz': 0.998, # [3], taken as 'CCSD(T)-F12b/VQZF12//CCSD(T)-F12a/TZF'
}
freq_level.replace('**', '(d,p)')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I understand, why do we want to use BAC from one basis set and freq scaling factor from a different basis?

@alongd
Copy link
Member Author

alongd commented Oct 29, 2019

I removed the b3lyp/6-31** commit, I think I haven't explained myself clearly. I'll make a new PR (#1791) for that. Rebased.

@mjohnson541 mjohnson541 merged commit 30553d7 into master Oct 30, 2019
@mjohnson541 mjohnson541 deleted the arkane_libs branch October 30, 2019 02:19
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants