WIP: RMG-Electrocat

[davidfarinajr]

This is a WIP PR for RMG-electrocat. The current goals of this PR are:

1. enable RMG to process charged species
2. add electrons - as Element/Atom/Molecule/Species or perhaps separate sub classes
3. add potential-dependent charge transfer reaction kinetics and thermochemistry
4. add proton/electron charge transfer reaction family (or families)
5. implement CHE model to estimate reaction energies and barriers of proton/electron reactions
6. add other method(s) to estimate thermo/kinetics for more general charge transfer reactions
7. add charged species to reaction families / new families for charges species
8. add data (standard reduction potentials, DFT calcs)
9. build electrochemical model as proof-of-concept (maybe CO2 reduction?)

Unanswered questions

1. How to include solvation effects of electrolyte?

• can we work with RMG's current solvation method, or do we need a new method?

2. Can we account for local effects of electric field (the double layer effects) on species and TS energies

[lgtm-com]

This pull request introduces 2 alerts when merging 8303ea1 into f3afdbf - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 2 alerts when merging e771940 into f3afdbf - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 2 alerts when merging 4cbd3c4 into f3afdbf - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 2 alerts when merging c01dd20 into f3afdbf - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 2 alerts when merging 7386926 into 1e21621 - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 2 alerts when merging b84681e into 1e21621 - view on LGTM.com

new alerts:

• 1 for Except block handles 'BaseException'
• 1 for Unused import

[lgtm-com]

This pull request introduces 6 alerts when merging d21a9d6 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging f8ebdec into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 050e5f8 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 1689be1 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging dca2015 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 70ed641 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 3a3031a into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 235bb03 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging bb32d7e into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 5384de3 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 6 alerts when merging 3f64812 into 1e21621 - view on LGTM.com

new alerts:

• 3 for Unused import
• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 11 alerts when merging 6234f31 into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 11 alerts when merging 0c19f20 into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[codecov]

Codecov Report

Merging #2000 (a4f06da) into master (6932bc8) will decrease coverage by 0.32%.
The diff coverage is n/a.

❗ Current head a4f06da differs from pull request most recent head 091fe9b. Consider uploading reports for the commit 091fe9b to get more accurate results

@@            Coverage Diff             @@
##           master    #2000      +/-   ##
==========================================
- Coverage   47.83%   47.50%   -0.33%     
==========================================
  Files         102       88      -14     
  Lines       27116    23304    -3812     
  Branches     6957     6062     -895     
==========================================
- Hits        12971    11071    -1900     
+ Misses      12752    11072    -1680     
+ Partials     1393     1161     -232     

Impacted Files	Coverage Δ
arkane/ess/qchem.py	72.72% <0.00%> (-2.95%)	⬇️
rmgpy/thermo/thermoengine.py	81.15% <0.00%> (-2.90%)	⬇️
rmgpy/data/transport.py	61.74% <0.00%> (-2.19%)	⬇️
arkane/encorr/data.py	92.34% <0.00%> (-1.92%)	⬇️
rmgpy/data/rmg.py	70.90% <0.00%> (-1.51%)	⬇️
rmgpy/data/kinetics/library.py	41.28% <0.00%> (-1.07%)	⬇️
arkane/ess/adapter.py	75.00% <0.00%> (-0.87%)	⬇️
arkane/ess/molpro.py	58.71% <0.00%> (-0.45%)	⬇️
rmgpy/constraints.py	94.33% <0.00%> (-0.40%)	⬇️
rmgpy/rmg/main.py	22.51% <0.00%> (-0.11%)	⬇️
... and 38 more

---

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 336273d...091fe9b. Read the comment docs.

[lgtm-com]

This pull request introduces 11 alerts when merging ce8c958 into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 11 alerts when merging d601bac into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 11 alerts when merging 9d05913 into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 11 alerts when merging e17bdef into a4f06da - view on LGTM.com

new alerts:

• 6 for Unused import
• 4 for Unused local variable
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 9 alerts when merging 6c26a6c into a4f06da - view on LGTM.com

new alerts:

• 4 for Unused local variable
• 4 for Unused import
• 1 for Except block handles 'BaseException'

[lgtm-com]

This pull request introduces 3 alerts when merging 091fe9b into 336273d - view on LGTM.com

new alerts:

• 2 for Unused local variable
• 1 for Except block handles 'BaseException'

[JacksonBurns]

This PR seems abandoned and should likely be closed.

@davidfarinajr is this PR still in progress?

[davidfarinajr]

This PR seems abandoned and should likely be closed.

@davidfarinajr is this PR still in progress?

Nope, we can close this. Let's make sure not to delete this branch though in case this work is picked up in the future.

[JacksonBurns]

This PR seems abandoned and should likely be closed.

@davidfarinajr is this PR still in progress?

Nope, we can close this. Let's make sure not to delete this branch though in case this work is picked up in the future.

Thanks for the quick reply!

Since we can always restore this branch if we decide to revive it, I will go ahead and delete it now for organization's sake.

[davidfarinajr]

This PR seems abandoned and should likely be closed.
@davidfarinajr is this PR still in progress?

Nope, we can close this. Let's make sure not to delete this branch though in case this work is picked up in the future.

Thanks for the quick reply!

Since we can always restore this branch if we decide to revive it, I will go ahead and delete it now for organization's sake.

fyi @rwest , this branch could be useful if electrocat work is resumed.

[rwest]

Thanks!
Most (but not all) of the commits seem to be in @mjohnson541's RMG-Electrochem PR #2316

Can you recall which of these are worth saving?

1. remotes/david/electrocat
2. remotes/david/electrocat_heyrovsky_bidentate_fix
3. remotes/david/electrocat_implicit_e_rebase
4. remotes/david/electrocat_implicit_electrons
5. remotes/david/electrocat_master
6. remotes/david/electrocat_rebase
7. remotes/official/electrocat
8. remotes/official/electrocat_bep
9. remotes/official/electrocat_heyrovsky
10. remotes/official/electrocat_rebase

[davidfarinajr]

Thanks! Most (but not all) of the commits seem to be in @mjohnson541's RMG-Electrochem PR #2316

Can you recall which of these are worth saving?

1. remotes/david/electrocat
2. remotes/david/electrocat_heyrovsky_bidentate_fix
3. remotes/david/electrocat_implicit_e_rebase
4. remotes/david/electrocat_implicit_electrons
5. remotes/david/electrocat_master
6. remotes/david/electrocat_rebase
7. remotes/official/electrocat
8. remotes/official/electrocat_bep
9. remotes/official/electrocat_heyrovsky
10. remotes/official/electrocat_rebase

oof, forgot I had that many branches 😆. I'm not sure, I'd go with the one that has the most recent commits. I do remember implicit_electrons was better than explicit implementation. If this project resumes, I could sort through these and offer suggestions and recommendations.