Codecov Report

Merging #1216 (7a1f478) into main (e6a5ffd) will increase coverage by 0.01%.
The diff coverage is 83.67%.

@@            Coverage Diff             @@
##             main    #1216      +/-   ##
==========================================
+ Coverage   65.42%   65.44%   +0.01%     
==========================================
  Files         320      320              
  Lines       46247    46321      +74     
  Branches    19655    19688      +33     
==========================================
+ Hits        30259    30315      +56     
- Misses      13460    13475      +15     
- Partials     2528     2531       +3     

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@speth / @decaluwe ... this is ready for review.

PS: also referencing Cantera/enhancements#38. FWIW, if we can rule out electrochemical 'sticking' formulations (which I currently left in), it would be very easy to introduce a third ElectrochemicalRate<> template (in analogy to the already existing InterfaceRate<> and StickingRate<> templates).

@speth .. thank you for the prompt review! Before going over the PR, I wanted to respond to your larger question.

I had one other thought which is that CoverageData and CoverageBase have grown to encompass quite a bit more than coverage dependence for the rate. If there's a good reason for not breaking the electrochemical specialization out into a derived class, I'm wondering if there are at least more descriptive names for these classes that we could use.

I was struggling with this also. The single-most important reason for not creating a separate specialization was that I didn't want to block electrochemical 'sticking' reactions. If we can rule this out, then a ElectrochemicalRate<> template would be the way to go (but I am wary of the odd-ball mechanism that may want to use this formulation, and @decaluwe - in a brief comment - opined that it's technically possible to go that route despite having some serious misgivings).

I agree that CoverageBase may not be the best descriptor any longer. InterfaceData and InterfaceBase would be the logical nomenclature, even if it the latter sounds somewhat clunky? Suggestions welcome ... 😄

I agree that CoverageBase may not be the best descriptor any longer. InterfaceData and InterfaceBase would be the logical nomenclature, even if it the latter sounds somewhat clunky? Suggestions welcome ... 😄

What about InterfaceRateBase in the latter case, so it doesn't sound like it's a base class for Interface, which is something else.

@speth ... done!

I'm encouraged by seeing all of the deprecated methods in InterfaceKinetics, as a sign of how much simplification all of this refactoring will eventually bring there.

It was actually surprising to see that the implementation is actually not that complex - it was just scattered all over and hard to follow.