[Reactor] clarify FlowDevice interface

[ischoegl]

This PR seeks to clarify the FlowDevice interface; specifically, the implementation of Valve.set_pressure_coeff was non-intuitive (misleading function name and partially incorrect doc string)

Please fill in the issue number this pull request is fixing:

Fixes #666

(also: addresses comment in #587, and prepares fix needed for #665)

Changes proposed in this pull request:

• differentiate Valve::setValveCoeff from PressureController::setPressureCoeff
• introduce Valve::setValveLift for time-dependent valve opening/closing
• introduce Valve.set_valve_function to eliminate Valve.valve_coeff(Func1) being erroneously applied to Valve::setFunction
• introduce properties Valve.valve_coeff / PressureController.pressure_coeff in Cython interface and deprecate Valve.set_pressure_coeff / PressureController.set_pressure_coeff (which is consistent with coefficient handling used by other zeroD objects)
• deprecate corresponding function calls in clib interface
• deprecate FlowDevice.setParameters (which was only used by MATLAB interface)

[codecov]

Codecov Report

Merging #667 into master will decrease coverage by 0.03%.
The diff coverage is 51.8%.

@@            Coverage Diff             @@
##           master     #667      +/-   ##
==========================================
- Coverage   70.66%   70.62%   -0.04%     
==========================================
  Files         372      372              
  Lines       43507    43564      +57     
==========================================
+ Hits        30744    30768      +24     
- Misses      12763    12796      +33

Impacted Files	Coverage Δ
src/clib/ctreactor.cpp	6.48% <0%> (-0.52%)	⬇️
include/cantera/zeroD/flowControllers.h	87.09% <100%> (+15.66%)	⬆️
src/zeroD/FlowDevice.cpp	94.44% <100%> (+0.32%)	⬆️
include/cantera/zeroD/FlowDevice.h	33.33% <6.66%> (-14.5%)	⬇️
src/zeroD/flowControllers.cpp	94.11% <95%> (-1.72%)	⬇️
src/transport/GasTransport.cpp	90.58% <0%> (+0.2%)	⬆️

---

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 7523022...1748996. Read the comment docs.

[speth]

I think having valve flow rate be a function of both time and the pressure difference will be a nice addition. The implementation looks good to me. I just had a few comments, mainly about the terminology used for the new functions.

[ischoegl]

@speth ... thanks for the review! Your comment and suggestion regarding the naming convention led me to clarify further, and I believe the interface is even clearer now: using a coefficient plus set_time_function and set_pressure_function there is no ambiguity left. Internally, all FlowDevice objects use the same approach (i.e. coefficient multiplied with function(s)). I also tested associated changes of clib from Matlab.

You are correct that those changes were necessary to properly address #665.

On a separate note: on my last merged PR (#660) for some reason my author tag was overwritten (no idea how a rebase/fixup may cause this: a squash is the likely culprit). The effect is that the commits aren't associated with @ischoegl: the associated sha's are d263566 and 77295b2. I can't fix this myself at this point, but I'm also not sure whether it matters and/or that it needs to be changed.

[speth]

@ischoegl The issue with commit ownership applies to this PR as well -- the "commiter" will get reset to me (or whoever merges this PR), but the "author" field will be preserved. The problem is that your "author" information is using your GMail address, while the "committer" field is using your LSU address (which I guess is your primary address on GitHub). After updating your Git configuration, you can reset this for this commit by doing git commit --amend --reset-author.

[ischoegl]

Thanks for the clarification: I will fix that momentarily (both here and for #676). It's highly confusing as my commits still show up with my tag.

[ischoegl]

Alright. I changed my primary address to my work and made sure that the commit shows the same. (Until now, I had my primary address set to gmail, which I used to set my account up years ago.) Let's see what happens ... .
EDIT: mystery solved ... typo in e-mail address (!)

[speth]

The one remaining to-do for this change is to update the "science" documentation for the reactor model on cantera.org (https://cantera.org/science/reactors.html#reactor-networks-and-devices).

[ischoegl]

@speth ... I can tackle this, but need to know how the docs are generated? Is there a primer you can direct me to?

[bryanwweber]

The restructuredText documents are stored in the Cantera/cantera-website repo. That file will be in the pages/science/reactors.rst I think (sorry no links, I'm on my phone).

[ischoegl]

@bryanwweber ... when is an appropriate time to change the cantera website for features only implemented in the development branch?

[bryanwweber]

That's a great question. I think the best thing to do is mark the updates as "development version only", to the extent that you can do that without deleting existing content. Anything that will be removed should be marked as deprecated or changed somehow. Let's try that out and see how it goes.

[ischoegl]

@bryanwweber ... would it make sense to create a branch that holds information until 2.5 goes live? (there are obviously multiple ways of handling this)

[bryanwweber]

I think (maybe?) it would make sense for the information to be deployed now, but clearly marked as for the development version only. But let's move this conversation over to the website repo.