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

Lithium #605

Open
wants to merge 110 commits into
base: main
Choose a base branch
from
Open

Lithium #605

wants to merge 110 commits into from

Conversation

mjohnson541
Copy link
Contributor

Contains all modifications of the database adding data for lithium species and reactions.

davidfarinajr and others added 24 commits March 29, 2024 11:50
since desorbed mols are liquid phase, change in entropy with adsorption to surface should be less
Abraham and Mintz parameters fitted using COSMO-RS data and solute parameter database
viscosity parameters taken from DEC
Surface Carbonate CO 2F Decomposition is
left out because it results in 4 products which
RMG currently cannot handle
this includes the CCCBDB data and the calculations
for wb97x-d3/def2-tzvp and
ccsd(t)f12/ccpvdzf12//wb97x-d3/def2-tzvp

the calculations for ccsd(t)f12/ccpvdzf12//wb97x-d3/def2-tzvp
for Li2O are commented out because while valid they were not
used in the BDE calculation because they create significant disagreement
on the Li-O BDE value with LiOH and the LiOH Li-O bond is more similar to
expected Li-O bonds of interest
since best not used for BACs while
we have so limited Li data and LiOH is better choice
as a result of the changes in the AECs
the H298 values (after AECs) for the
calculated data has changed for the
corresponding levels of theory
@ssun30
Copy link
Contributor

ssun30 commented Apr 11, 2024

Hi Matt, I've been trying to have RMG-electrochem to work for the electrocatalytic CO2 reduction reaction. David has done some work on this for his thesis and had this branch: https://github.com/davidfarinajr/RMG-database/tree/electrocat_master. It seems that all three commits on this branch have been merged into the current lithium branch. Several reaction families, such as Surface_Proton_Electron_Reduction_Alpha_vdW, are necessary for the proton-coupled electron transfer (PCET) reactions. But in this commit: 1105f70 these families are removed because they don't work. And the non-vdW surface proton-electron reduction families such as Surface_Proton_Electron_Reduction_Alpha could not generate any reactions in the model edge.

One thing I've noticed is that on David's branch for RMG-Py: https://github.com/davidfarinajr/RMG-Py/tree/electrocat_master, he developed an ElectrodeReactor type that is not incorporated in the echem-rebase branch. That reactor model simulates the gas-solid interface but our end goal, like for the Li-ion battery project, is to generate models at the liquid-solid interface. What kind of work is required to adapt David's proposed reaction families to the LiquidSurfaceReactor that is currently used for electrochemistry modelling? Or is there any later developments that replaced these families for PCET reactions?

@mjohnson541
Copy link
Contributor Author

So earlier this year I set aside two weeks to finish the work and paper associated with this branch, when I was debugging a number of the PCET families weren't working and weren't important to the electrolyte degradation system I was modeling. I wasn't aware anyone else was planning to do anything in this area and I figured rewriting them later was going to be about as much work as debugging them (due to changes during development) so I removed them. "ElectrodeReactor" isn't ringing any bells for me, I'm not familiar with it and I don't think it was ever part of the branch.

There's no system that explicitly replaces those PCET families and I don't think there are any structural barriers that should stop them from being added. You should just be able to just pluck the family files and debug them, try to model them after the working surface electrochemical families.

@mjohnson541
Copy link
Contributor Author

To clarify if you're not modeling the gas phase conditions I don't think there's any reason to not use LiquidSurfaceReactor.

@ssun30
Copy link
Contributor

ssun30 commented May 17, 2024

Hi Matt, I have several questions regarding the ArrheniusChargeTransfer kinetics class. It has an attribute V0 that seems to always be 0 volt. 1) What is the definition of this value and in what circumstances should it be a non-zero value? Example:

entry(
    index = 2,
    label = "Root_1NO->O",
    kinetics = ArrheniusChargeTransfer(A=(1.30887,'m^3/(mol*s)'), n=2.52389, Ea=(51.775,'kJ/mol'), V0=(0,'V'), alpha=0.0625, electrons=1, T0=(1,'K'), uncertainty=RateUncertainty(mu=-2.249383619443048e-16, var=2.336282225810139, Tref=1000.0, N=8, data_mean=0.0, correlation='Root_1NO->O',), comment="""BM rule fitted to 8 training reactions at node Root_1NO->O
    Total Standard Deviation in ln(k): 3.0642178763170635"""),
)

For the CO2RR families, I used an older class SurfaceChargeTransfer made by David and it also has this attribute. But in his work there is a function get_reversible_potential which sets V0 to the potential at which the charge transfer reaction is at equilibrium. 2) Is there a difference in how these two 'V0s' are defined in these different classes?

Also, 3) would you recommend converting all SurfaceChargeTransfer reactions into ArrheniusChargeTransfer in the database? Or is there a fundamental difference between the two?

@mjohnson541
Copy link
Contributor Author

Hi Matt, I have several questions regarding the ArrheniusChargeTransfer kinetics class. It has an attribute V0 that seems to always be 0 volt. 1) What is the definition of this value and in what circumstances should it be a non-zero value? Example:

entry(
    index = 2,
    label = "Root_1NO->O",
    kinetics = ArrheniusChargeTransfer(A=(1.30887,'m^3/(mol*s)'), n=2.52389, Ea=(51.775,'kJ/mol'), V0=(0,'V'), alpha=0.0625, electrons=1, T0=(1,'K'), uncertainty=RateUncertainty(mu=-2.249383619443048e-16, var=2.336282225810139, Tref=1000.0, N=8, data_mean=0.0, correlation='Root_1NO->O',), comment="""BM rule fitted to 8 training reactions at node Root_1NO->O
    Total Standard Deviation in ln(k): 3.0642178763170635"""),
)

For the CO2RR families, I used an older class SurfaceChargeTransfer made by David and it also has this attribute. But in his work there is a function get_reversible_potential which sets V0 to the potential at which the charge transfer reaction is at equilibrium. 2) Is there a difference in how these two 'V0s' are defined in these different classes?

Also, 3) would you recommend converting all SurfaceChargeTransfer reactions into ArrheniusChargeTransfer in the database? Or is there a fundamental difference between the two?

  1. So in general for these reactions: Ea = Ea0 - alphaN_electronsF*(V-V0)
    In practice most calculations I ran were at V0=0, but if you were to pull data from experiments it’s unlikely you would have V0=0. I didn’t add get_reversible_potential because I didn’t have a reason to use that as a reference over V0=0, the choice of reference should not impact the evaluations.

  2. I don’t believe there is a difference in V0 treatment between the two classes.

  3. I think it would be good to only use one class…as far as I can tell the only reason the “surface arrhenius” classes exist is to attach coverage_dependence parameters…which we probably want to do long term…if we want to stick with the same architecture it would be best to instead have SurfaceChargeTransfer inherit from ArrheniusChargeTransfer and add in the coverage_dependence attribute as in SurfaceArrhenius.

@rwest
Copy link
Member

rwest commented Aug 15, 2024

Hi @mjohnson541 how do we deal with this PR with 110 commits and 557 files?
There are a lot of conflicts.

Do we just pick out what needs to change to make electrochemistry function, and wave goodbye to some changes?
Do we squash a lot of stuff and wave goodbye to some history (many of the commit messages aren't particularly informative anyway)?
Pick out what we need for electrochemistry, and leave the rest for you to deal with?
Talk through it with you and figure out how to divide and conquer?

@ssun30 aren't trying to model lithium, but we do need to get electrochemistry (eg. CO₂ reduction) functioning on a modern main.

@mjohnson541
Copy link
Contributor Author

The conflicts listed here are almost entirely for reference set data which impacts Arkane, but not RMG runs (the only exception is the recommended file, which should be easy to deal with).

We should get those in eventually, but those should be able to be easily separated from the RMG vital changes.

@mjohnson541
Copy link
Contributor Author

I can try and dig through this relatively soon, but I don't have time right now.

This was referenced Sep 19, 2024
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.

4 participants