First Cython implementation of ReactorNet::advance_to_steady_state

[thomasfiala]

Hi, this is a first functional Cython implementation of a new function advance_to_steady_state.

Could you please have a look at it and tell me if that's a way to go?
You could test it e.g. by running this python script:

import cantera as ct
gas = ct.Solution('h2o2.xml')
gas.TPX = 1300, 1e5, 'H2:1,O2:.5'
r = ct.Reactor(gas)
sim = ct.ReactorNet([r])
res = sim.advance_to_steady_state(write_residuals=True)

Thanks!

[speth]

Thomas, this looks interesting, and certainly simplifies a common use case. A few thoughts:

• It might make sense to split into two commits: one to introduce getState and deprecate getInitialConditions, and the second to implement advance_to_steady_state.
• I was somewhat surprised at how well the termination algorithm appears to work in conjunction with the dynamic step size used by the integrator. I had been a bit worried that there would be some cases where the integrator would take very small steps and then incorrectly terminate the search.
• Since Python code is comparatively slow, you can get to the steady state faster if you call self.step() several times (say, 10) between each check of the termination condition.
• I think the residual should be divided by sqrt(self.n_vars)
• The residual calculation can be written as np.linalg.norm((state - previous_state) / (max_state_values + smallest_value))
• The return array of residuals should be truncated, i.e. return residuals[:step]
• Would it make sense to refer to residual_threshold and smallest_value as rtol and atol? Does it make sense to use the already-defined properties of the reactor network for this? They seem to be used in approximately the same way.
• I think I would rename write_residuals to return_residuals. Also, in the docstring for this parameter, replace yields with returns, as yield has a special meaning in Python.
• Watch out for trailing whitespace being added in a few places (git diff and git show can help identify this)
• Is the get_state method of the Python Reactor class useful? I'm not sure where it would be used that it wouldn't be better to use ReactorNet.get_state.
• Don't make is comparisons with numeric constants, e.g. 0. Either use ==, or in the specific case of 0, just if not var:.

[thomasfiala]

Ray, thanks a lot for your comments! I'll implement your ideas next Monday.
Just a thought about the multiple commits: Does it make more sense to create separate pull requests, or can I simply split the commit and then re-upload it to here (if that's possible)? Thanks!

[bryanwweber]

@thomasfiala You can force push the new commits to your branch reactor_steady_state, and they will automatically show up here since that branch is already connected to this pull request. You'll have to force push (git push -f ...) if you go back and rebase or rewrite some of the old commits because git doesn't like to overwrite things on remotes.

[thomasfiala]

Hi, I tried to implement all of your comments and split the single commit into several pieces. I also added a test case and updated the tutorials.

Regarding Ray's thoughts:

• I still kept the get_state method for the reactor class. Although I agree with you that there probably is little use for this function, I think that it might be helpful for people to understand the function component_index.
• I used atol instead of smallest_value. However, I refrained from substituting residual_threshold with rtol, because these values have to be different. residual_threshold has to be larger than rtol, because otherwise the time integration fails. Nevertheless, I modified the default method to use 10 * rtol for the residual_threshold.