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

Implement Butler-Volmer for Electrochemical Reactions #44

Open
korffdm opened this issue Apr 2, 2020 · 10 comments
Open

Implement Butler-Volmer for Electrochemical Reactions #44

korffdm opened this issue Apr 2, 2020 · 10 comments
Labels
feature-request New feature request

Comments

@korffdm
Copy link

korffdm commented Apr 2, 2020

Abstract

Add a reaction type for Butler-Volmer electrochemical reactions. Include ability for users to use a simplified B-V (i.e. Tafel) form, user specified reaction orders, calculation of overpotential based on current state, and flexibility in i_o.

Motivation

This will add options for electrochemical reactions and could include features that allow the user to query for information they might desire (such as an overpotential).

@korffdm korffdm added the feature-request New feature request label Apr 2, 2020
@decaluwe
Copy link
Member

decaluwe commented Apr 14, 2020

Thanks, @korffdm. FYI, this issue should link to #38, which discusses the issue pretty comprehensively. I would suggest that maybe we take bite-sized chunks out of that larger issue and discuss the implementation, one at a time.

So what I would suggest for this issue is to:

  • Make this about one specific aspect of that larger issue (e.g. "Enable direct use of the Butler-Volmer reaction form.")
  • Convert it from feature-request to work-in-progress.
  • Lay to your vision, here, for how that implementation will look.
  • Implement the changes and make sure your PR, when ready, references this issue.

@speth, just a heads up - @korffdm and I were looking at this, today, and there are aspects of the current butler-volmer infrastructure that we may want to "un-depcrecate."

@decaluwe
Copy link
Member

We are currently considering creating a butler-volmer reaction type, as a child class of InterfaceKinetics.

Rather than the current (partially-implemented and deprecated) butler-volmer reaction type, which handles everything via flags, we think this would be a cleaner and easier-to-follow implementation.

However, before we go too far down the path of developing an implementation, we wanted to just put the idea out there, in case anyone (@speth and @bryanwweber) sees any issues or has a preference for a different approach.

Thanks.

@ischoegl
Copy link
Member

ischoegl commented May 25, 2020

@decaluwe ... while I have no comments on the implementation itself, I'd suggest to add this after 2.5 is released?

Other than that, reactions are currently not handled by factory classes which may make sense to implement before. I had some suggestions in Cantera/cantera#745 , but there are certainly other options.

@decaluwe
Copy link
Member

Thanks, @ischoegl -- while we can wait for PR after 2.5, the functionality is desired right now, so whether or not we start a PR, we would develop the changes in our own version, for ourselves and others (@wbessler and collaborators) to use more immediately.

Thanks for reminding me about the reaction factory, as I had not taken the time to fully understand it, yet, and want to do so.

@ischoegl
Copy link
Member

@decaluwe ... I’m generally applauding the effort.

The issue is really about release cycles: the current development version is far ahead of 2.4 already; I’d lobby for clearing out the current pipeline, and hope for a faster release cycle thereafter?

@decaluwe
Copy link
Member

@ischoegl understand and agree.

@bryanwweber
Copy link
Member

@decaluwe @ischoegl Creating a PR for these features wouldn't delay releasing 2.5.0. Please create the PR as soon as you're able to share the code!

If the inheritance from InterfaceKinetics helps simplify the implementation, I don't see a problem with doing that. We'll just need to be clear in our messaging about the changes, for anyone who was using the deprecated features.

@decaluwe
Copy link
Member

decaluwe commented Jun 3, 2020

Thanks @bryanwweber -- thinking about it some more, I don't think inheritance would work. What we want is a new reaction type that is still managed by InterfaceKinetics. A child class would instantiate an entirely new kinetic manager - is that correct?

This would be similar to how falloff reactions, for example are still handled within GasKinetics. The difference here being that the ROP formalism is still pretty much the same, in that case. For a Butler-Volmer Reaction, we will basically need to have a flag that diverts most of updateROP. I suppose that is not too bad, just maybe not as elegant as I would like :)

Thanks again.

@ischoegl
Copy link
Member

ischoegl commented Jun 4, 2020

As a minor comment, a lot of frequently used / computationally heavy routines in the cantera gas phase kinetics core make use of templated functions, where you can replace code as long as interfaces fit the mold. I haven’t spent much time on interfacekinetics, though, so I’m not sure that this approach is consistently followed.

@decaluwe
Copy link
Member

Thanks, @ischoegl -- I'll take a look at it and see if I can figure out whether something similar is workable here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature request
Projects
None yet
Development

No branches or pull requests

4 participants