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

Marcus Kinetics #247

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

Marcus Kinetics #247

wants to merge 57 commits into from

Conversation

mjohnson541
Copy link
Collaborator

Adapt RMS to handle Marcus kinetics particularly adapting to having forward rate coefficients dependent on dGrxn and d.

@ssun30
Copy link
Contributor

ssun30 commented Mar 29, 2024

Hi Matt, what's the status of this PR? Could you rebase this to the main branch? I tried rebasing on my own and resolving a bunch of conflicts (mostly related to formatting) but something broke.

@mjohnson541
Copy link
Collaborator Author

This one is waiting until after v1.0.0. It has things needed for RMG Electrochem. I just rebased it on main.

@ssun30
Copy link
Contributor

ssun30 commented Sep 26, 2024

Hi @mjohnson541 , I saw that this branch has been rebased onto PR #259 which includes the changes to the new PythonCall.jl interface from PR #256 . So to merge this PR we would need to deploy both PR #256 and its twin PR on the RMG-Py side first. Is that correct?

Our echem-rebase branch is still based onto the main branch of RMG-Py that still uses the old PyCall.jl interface and it wouldn't work with the marcus branch in its current state (locally and CI). Our hope is that echem-rebase and lithium could be merged before the Python-Julia overhaul is required. Is there any particular changes in this PR that requires the new interface?

@ssun30
Copy link
Contributor

ssun30 commented Oct 15, 2024

Hi @mjohnson541. I think we are closing in on merge Electrocat into RMG. The RMG-Py and RMG-database branches are now both ready to deploy. Our plan is to temporarily run future continuous integrations on this branch until we finish the RMG-RMS interface overhaul.

The marcus_development branch is kept up-to-date with this PR minus the PythonCall.jl part. Currently there are two unit tests that failed due to changes to Domains and Interface. Any thoughts on whether the new evaluated values are expected or not?

Test failure 1: Vapor-liquid phase multi-domain reactor simulation with VaporLiquidMassTransferInternalInterfaceConstantT and VolumeMaintainingOutlet interface. The test failed at:

@test sol(sol.t[end])[ind] ≈ 0.11758959354431776 rtol = 1e-5 #test there are oxygen dissolved into the liquid

Evaluated: 6.239657308203088e6 ≈ 0.11758959354431776 (rtol=1.0e-5)

Test failure 2: Multi-domain Gas-Surface ConstantV and ConstantTAPhi Simulation. The test failed at:

@test molefractions(ssys.sims[1], "H2O", 1e-3) ≈ 0.12732345278036702 rtol = 1e-5

Evaluated: 0.12990076889532717 ≈ 0.12732345278036702 (rtol=1.0e-5)

@mjohnson541
Copy link
Collaborator Author

Yeah, I don't think those are expected, it looks like I hadn't figured why those were occurring yet.

@ssun30 ssun30 mentioned this pull request Nov 6, 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.

3 participants