Skip to content

Remove duplicate training reaction from diels alder#588

Merged
xiaoruiDong merged 2 commits into
mainfrom
diels_alder
Sep 9, 2022
Merged

Remove duplicate training reaction from diels alder#588
xiaoruiDong merged 2 commits into
mainfrom
diels_alder

Conversation

@kspieks

@kspieks kspieks commented Jun 10, 2022

Copy link
Copy Markdown
Contributor

Following up on PR #582, training reaction 19 that I had added actually matches training reaction 0. Since reaction 0 was calculated from rate rules from 300 K to 1500 K, @hwpang and I agree that we should replace this duplicated reaction with the new value calculated at CCSD(T)-F12a/cc-pVDZ-F12//ωB97X-D3/def2-TZVP from 300 K to 2000 K. After replacing this reaction, I re-ran ATG to refit the rate rules. Doing so slightly decreased the median error from 6.14 to 5.95.

@kspieks kspieks requested a review from hwpang June 10, 2022 19:27
@kspieks kspieks self-assigned this Jun 10, 2022
@xiaoruiDong xiaoruiDong self-requested a review September 8, 2022 18:11

@xiaoruiDong xiaoruiDong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Please rebase the change.

These training reactions were duplicated. However, the first instance came from rate rules and was only valid for smaller T range so we decided to replace it with the more accurate TST calculation
@kspieks

kspieks commented Sep 8, 2022

Copy link
Copy Markdown
Contributor Author

@xiaoruiDong Just rebased and pushed. Thanks for reviewing!

@xiaoruiDong xiaoruiDong merged commit 0b3d04e into main Sep 9, 2022
@xiaoruiDong xiaoruiDong deleted the diels_alder branch September 9, 2022 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants