RMG-Py
RMG-Py copied to clipboard
Isodesmic improvements
Motivation or Problem
There are a few improvements I made to the isodesmic reactions code in RMG as part of the work on an upcoming BACs paper. With these improvements, isodesmic reactions show similar performance to BACs under certain conditions.
Description of Changes
The biggest change is adding in higher order isodesmic schemes
Testing
I used this branch to generate data/plots for the upcomming BACs paper. I can share a talk on this to help with the review.
This pull request introduces 1 alert when merging 038bca4152d16dab8063edcca52774b5a9d3947f into 47c4e0b16ee274919ec24aa4160ac5e83cc9f0a9 - view on LGTM.com
new alerts:
- 1 for Unused import
At a first look, this PR looks good. As minor changes:
Codecov Report
Merging #2217 (4259d1e) into main (1d67d94) will decrease coverage by
0.82%
. The diff coverage is83.50%
.
:exclamation: Current head 4259d1e differs from pull request most recent head 5e8919b. Consider uploading reports for the commit 5e8919b to get more accurate results
@@ Coverage Diff @@
## main #2217 +/- ##
==========================================
- Coverage 48.49% 47.67% -0.82%
==========================================
Files 110 105 -5
Lines 30758 28275 -2483
Branches 8039 7479 -560
==========================================
- Hits 14917 13481 -1436
+ Misses 14318 13343 -975
+ Partials 1523 1451 -72
Impacted Files | Coverage Δ | |
---|---|---|
arkane/encorr/isodesmic.py | 79.44% <82.81%> (-0.98%) |
:arrow_down: |
arkane/encorr/reference.py | 71.32% <100.00%> (+0.86%) |
:arrow_up: |
... and 50 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
This pull request is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant pull request, otherwise it will automatically be closed in 30 days.
@JacksonBurns this PR should make its way into RMG at some point. If you want I can take one last look at it, update it to main, and then merge it in. Also happy to wait for approval if someone on the team wants to take over as reviewer. @oscarwumit is working on the manuscript for this, so he might be the most familiar person there.
@amarkpayne thanks for the update! I am going to resolve the conflicts in the environment.yml
file and see if this is still close to merging. Please give it another look once the CI gets underway.
This pull request is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant pull request, otherwise it will automatically be closed in 30 days.
In the time since this PR was opened, the new pyutillib
dependency has ceased development. Refactoring this PR to deal with this is currently not in the cards, so I'm going to remove the stale and abandoned labels and soft reject this PR.