Update reactor API

[ischoegl]

Changes proposed in this pull request

Make Python API more pythonic

• use properties instead of of set_XYZ methods in Python
• evaluate functors at reactor network time
• replace set_master by primary property

API becomes consistent with existing mfc behavior

>>> mfc.mass_flow_rate = 0.3
>>> mfc.mass_flow_rate = lambda t: 2.5 * exp(-10 * (t - 0.5)**2)

If applicable, fill in the issue number this pull request is fixing

Closes Cantera/enhancements#160
Closes #1460

If applicable, provide an example illustrating new features this pull request is introducing

net.initial_time = 0.                   # instead of net.set_initial_time(0.)
wall.velocity = lambda t: sin(t)        # instead of wall.set_velocity(lambda t: sin(t))
wall.heat_flux = lambda t: cos(t)       # instead of wall.set_heat_flux(lambda t: cos(t))
vdot = wall.expansion_rate              # instead of wall.vdot(net.time)
qdot = wall.heat_rate                   # instead of wall.qdot(net.time)
valve.time_function = lambda t: sin(t)  # instead of valve.set_time_function(lambda t: sin(t))
mfc.pressure_function = lambda p: p**2  # instead of mfc.set_pressure_function(lambda p: p**2)
pc.primary = mfc                        # instead of pc.set_master(mfc)

In clib, the new functions wall_vdot2 and wall_qdot2 are introduced to preserve current behavior for the old MATLAB toolbox (i.e. no backport) while freeing up wall_vdot and wall_qdot for all revised interfaces (i.e. reactor network time is automatically used). Edit: no change for traditional MATLAB, new functions wall_expansionRate and wall_heatRate for revised interfaces.

Checklist

• The pull request includes a clear description of this code change
• Commit messages have short titles and reference relevant issues
• Build passes (scons build & scons test) and unit tests address code coverage
• Style & formatting of contributed code follows contributing guidelines
• The pull request is ready for review

[codecov]

Codecov Report

Merging #1515 (781d41d) into main (b4dad59) will decrease coverage by 0.08%.
The diff coverage is 47.68%.

@@            Coverage Diff             @@
##             main    #1515      +/-   ##
==========================================
- Coverage   70.59%   70.51%   -0.08%     
==========================================
  Files         376      376              
  Lines       58956    59072     +116     
  Branches    21197    21217      +20     
==========================================
+ Hits        41619    41657      +38     
- Misses      14264    14339      +75     
- Partials     3073     3076       +3     

Impacted Files	Coverage Δ
include/cantera/numerics/Func1.h	49.55% <ø> (ø)
include/cantera/zeroD/Reactor.h	61.76% <ø> (ø)
src/clib/ctreactor.cpp	5.97% <0.00%> (-0.28%)	⬇️
include/cantera/zeroD/Wall.h	46.15% <33.33%> (-2.69%)	⬇️
src/zeroD/flowControllers.cpp	65.90% <33.33%> (-6.60%)	⬇️
interfaces/cython/cantera/reactor.pyx	83.04% <40.90%> (-6.58%)	⬇️
src/zeroD/Wall.cpp	51.78% <60.00%> (-28.86%)	⬇️
include/cantera/zeroD/flowControllers.h	84.84% <66.66%> (ø)
src/zeroD/FlowDevice.cpp	84.78% <77.77%> (-1.71%)	⬇️
include/cantera/zeroD/FlowDevice.h	73.68% <100.00%> (+4.93%)	⬆️
... and 4 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[bryanwweber]

Thanks Ingmar! The master thing has been a wart for a while now 😊

[ischoegl]

@bryanwweber ... Thanks for the review! I addressed one of your comments (and also made sure that the remaining new getters are consistent); I am hoping to convince you about the other suggested change.

Edit: Also added a (trivial) fix for #1460 / caught a reference to 'master' in the new Matlab toolbox.

[speth]

Thanks for these updates, @ischoegl. I think a lot of this makes sense, and I appreciate the work towards making the different language interfaces more consistent.

I am somewhat ambivalent about the increased level of magical behavior for these properties where the setter takes a function but the getter just returns a value, but as you note we've already made a change that direction with MassFlowController.mass_flow_rate, so at least it's increasing consistency.

[ischoegl]

@speth ... I believe I took care of everything, where I opted for heat_rate and expansion_rate to replace Q/qdot and vdot. Note that I applied the suggested updates to ExtensibleReactor also. In addition, I noticed the updates you had for DeprecationWarnings in #1519, which I applied preemptively here.

Overall, the deprecations got a lot simpler, where an intermittent "3" suffix is not needed.

I am somewhat ambivalent about the increased level of magical behavior for these properties where the setter takes a function but the getter just returns a value, but as you note we've already made a change that direction with MassFlowController.mass_flow_rate, so at least it's increasing consistency.

Agreed, but I think it is ultimately useful and intuitive. We have other properties that return values without having a setter, so it is in a way consistent from that perspective as well.

[bryanwweber]

@ischoegl @speth My only remaining suggestion here is to use volumetric_expansion_rate instead of expansion_rate to be explicit. I don't feel strongly though, as it is very wordy.

[speth]

Thanks, @ischoegl. Just a few minor issues to resolve.

Regarding, @bryanwweber's suggestion, I think I lean slightly toward just expansion_rate, given the somewhat excessive length of volumetric_expansion_rate.

[ischoegl]

Thanks for catching those items, @speth. I corrected them all.

Regarding, @bryanwweber's suggestion, I think I lean slightly toward just expansion_rate, given the somewhat excessive length of volumetric_expansion_rate.

I tend to agree.

[speth]

Looks good to me!

[ischoegl]

Looks good to me!

Thanks! will rebase as #1521 was just merged, so the one new instance of TabulatedFunction won't cause trouble.

[ischoegl]

@speth ... could you reapprove?