Codecov Report

Merging #1181 (547c737) into main (551a45c) will increase coverage by 0.04%.
The diff coverage is 85.23%.

@@            Coverage Diff             @@
##             main    #1181      +/-   ##
==========================================
+ Coverage   65.38%   65.42%   +0.04%     
==========================================
  Files         318      320       +2     
  Lines       46095    46247     +152     
  Branches    19604    19655      +51     
==========================================
+ Hits        30139    30259     +120     
- Misses      13442    13460      +18     
- Partials     2514     2528      +14     

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 551a45c...547c737. Read the comment docs.

@speth … thank you for the comments! I addressed all except …

The fact that the initial value doesn't correspond to the state of the Interface object is definitely surprising […]. Should we at least be updating the rate when you first access the Reaction object? Is there any useful way to tie this to the Kinetics object? I can think of some complicated ones, but I'm not sure they're worth it.

The behavior is definitely not what I expected either. I can think of something (updating when adding reactions), but my first tests were not successful. I need to make sure that the state of the phase is set before reactions are added … which may or may not be already the case (fwiw, I didn’t get the expected results). I‘ll try again later today.

Also, it seems that there is no longer a way to get the base value of this parameter, although I guess you can always get it by looking at the input_data, which might be fine.

This is correct, and using serialization here was exactly my rationale for eliminating a couple of methods.

[...] trying this out I noticed that the behavior of ArrheniusInterfaceRate.activation_energy and friends is very non-intuitive:

>>> iface = ct.Interface('ptcombust.yaml', 'Pt_surf')
>>> R = iface.reaction(1)
>>> print(R.rate.activation_energy)
67400000.0  # this is the base value with no coverage dependence
>>> R.rate(500, iface.coverages)
>>> print(R.rate.activation_energy)
64400000.0  # now this includes the coverage dependence

[...] Is there any useful way to tie this to the Kinetics object? I can think of some complicated ones, but I'm not sure they're worth it.

@speth ... you are probably aware of this, but the behavior is due to the fact that R.rate holds the original ReactionRate object that is created when the Reaction is first instantiated, which is separate from the copy that Kinetics uses to actually evaluate the rate. In my tests, I was partially successful in updating ReactionData right away (the best entry point is likely around the Kinetics::resizeReactions specializations), but at the moment there is no way of probing the content.

A resolution would require something akin to #1051, but I'm not sure that I want to include this in 2.6 this PR. There may be a way to copy the initial state over, but it would be imho more important to keep things synchronized at all times. PS: I opened #1211 as this goes well beyond the changes proposed in this PR.