Improve handling of max_time_step in ReactorNet

[ischoegl]

Enable checks for max_time_step from Python frontend, and clarify default setting in docstring.

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

Fixes #730

Changes proposed in this pull request:

• implement ReactorNet::maxTimeStep to allow for external queries
• implement getter/setter for property ReactorNet.max_time_step
• deprecate ReactorNet.set_max_time_step

Changing the default behavior would be simple by restricting m_maxstep here:

cantera/src/zeroD/ReactorNet.cpp

Line 22 in c149c4e

m_maxstep(0.0), m_maxErrTestFails(0),

but I am hesitant to do so.

[codecov]

Codecov Report

Merging #731 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   70.84%   70.84%   -0.01%     
==========================================
  Files         374      374              
  Lines       43666    43668       +2     
==========================================
+ Hits        30937    30938       +1     
- Misses      12729    12730       +1

Impacted Files	Coverage Δ
include/cantera/zeroD/ReactorNet.h	80% <100%> (+1.21%)	⬆️
src/transport/GasTransport.cpp	90.37% <0%> (-0.21%)	⬇️

---

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 a221c63...cb67182. Read the comment docs.

[bryanwweber]

Thanks @ischoegl. This looks good to me, I didn't even realize you couldn't query this value. One small change, I think.

[ischoegl]

@bryanwweber ... before making the requested change (which is minor). Do you have any comments on the default behavior (see #730). I believe that limiting the time step may resolve some of the undesirable behavior (very large time step with CVODE interpolation missing some potential changes).

[bryanwweber]

No, I think we should leave the default as-is. CVODES has a built-in way to determine the appropriate time step. I think it is on the user to determine for their use case if that needs to be limited somehow. I don't think we can pick a value that is both 1) large enough that we don't limit the CVODES unnecessarily and 2) small enough to actually effect any change.

[ischoegl]

I think we should leave the default as-is.

OK.