Steady-State Solver for ZeroD Reactors #1021
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1021 +/- ##
==========================================
+ Coverage 65.39% 65.89% +0.50%
==========================================
Files 318 320 +2
Lines 46109 47575 +1466
Branches 19601 20496 +895
==========================================
+ Hits 30153 31350 +1197
- Misses 13452 13686 +234
- Partials 2504 2539 +35
Continue to review full report at Codecov.
|
|
@paulblum ... as this just came up on the UG, what is the current status of this PR? - and to confirm: this should close Cantera/enhancements/issues/31, correct? |
|
@paulblum and @bryanwweber … this PR came up again in discussions recently. Are there any plans to push this over the finishing line? |
|
@ischoegl Thanks for following up. I don't personally have any plans to work on this, and @paulblum has moved on to a PhD program in a different area, unfortunately, so he probably won't have time either. If you want to pick up the ball from here, please feel free to reuse this code/commits as necessary, or ditch it altogether. |
|
@bryanwweber ... thanks for the clarification and too bad indeed. It's not something I was intending to work on (and don't have any immediate plans at least at the moment). |
There was a problem hiding this comment.
I rebased this onto the current main branch so it doesn't get left too far behind. The comments below are partly just to remind me (or whoever finds time to pick this up again) of a couple things that I saw that need some work in the implementation.
Currently, there are no tests, and the simplest thing I could think of to test didn't actually work. This was to use the mix1.py example and replace the call to sim.advance_to_steady_state() with a call to sim.solve_steady(). The solver prints out the message "Failure to converge in 17 steps.", and the solution is clearly not the right steady-state.
| for(int i = 0; i < MAX; i++) { | ||
| newtonsolves++; | ||
| m_config = &m_directsolve_config; | ||
| if(solve(m_x.data()) > 0) { | ||
| writelog("\nConverged in {} newton solves, {} timesteps.", newtonsolves, timesteps); | ||
| return 1; | ||
| } | ||
| m_config = &m_timestep_config; | ||
| copy(m_xsave.begin(), m_xsave.end(), m_x.begin()); | ||
| for(int j = 0; j < MAX; j++) { | ||
| if (solve(m_x.data()) < 0) { | ||
| writelog("\nTimestep failure after {} newton solves, {} timesteps.", newtonsolves, timesteps); | ||
| return -1; | ||
| } | ||
| timesteps++; | ||
| } | ||
| copy(m_x.begin(), m_x.end(), m_xsave.begin()); | ||
| } | ||
| writelog("Failure to converge in {} steps.", MAX); | ||
| return 0; |
There was a problem hiding this comment.
I think this solver loop only does a much more limited version of the hybrid time-stepping / steady solver structure that's implemented by the MultiNewton class, and is only alternating between solving the steady state and taking time steps of a fixed size. Given the sophistication of the MultiNewton algorithm, it might be worth seeing if we can pose this problem in a way that that solver can use directly.
| for (int i = 0; i < m_nv; i++) | ||
| { | ||
| m_newt->setBounds(i, -1.0e-3, 1.01); | ||
| } | ||
| m_newt->setBounds(0, 1.0e-3, 100); | ||
| m_newt->setBounds(1, 0, 5); | ||
| m_newt->setBounds(2, -10000000, 10000000); | ||
|
|
||
| m_newt->setConstants({1}); | ||
| // m_newt->setConstants({0,1,2,11,12}); |
There was a problem hiding this comment.
Settings bounds definitely needs to be moved to the individual reactor classes. I think these are meant to apply only to the base Reactor class, but even then the values for the first (mass) and second (volume) components don't quite make sense.
|
I have a relatively broad question here that follows up on #654 (comment), reproduced here:
Fundamentally, there's nothing wrong with the 1D approach, but at least from my experience, it is prone to getting stuck for problems that involve marginal (low heating value) mixtures. But if I am informed correctly, the basic idea for the hybrid solver predates Cantera, and is essentially what was implemented for the original CHEMKIN. My main concern here is that we may be implementing 1980's ideas (which still may be worth keeping, but a lot has happened since) ... why not use CVODES for the transient solver, especially if we will be able to precondition soon? Are there better approaches than a standard newton solver? (e.g. CG etc.). Can we leverage analytical jacobians (which we are close to finally having after #1089 and #1010)? Can we leverage packaged root finders? Can we use some findings here for updated approaches to the 1D solver down the road? |
My understanding is that we simply don't need all the overhead of creating CVODES stuff, either the time or memory overhead, since the controls that CVODES provides are simply unnecessary. Likewise for analytical Jacobians.
Paul originally attempted KINSOL here, but packaged solvers don't tend to provide control on constraints (e.g., mass fractions) that are required for this application. They also don't provide the hybrid approach, and a Newton algorithm is fairly simple to implement, and it's already done for us and been validated for ~20 years... Again, just my understanding.
This looks like great enhancement request if you can provide some more details 😄 As this implementation is relatively close to done, I don't think we should close this, unless replacement features are merged first though.
Sure, that seems natural... |
This sounds like a bug, or something that could be improved more generally. Opening an issue with a sample script that fails would be most welcome! |
|
@bryanwweber … would Cantera/enhancements#154 help with the KINSOL issues? |
|
@ischoegl I don't think so. You still have constraints on the state variables to be positive, so that wouldn't be resolved. You'd also have to run the hybrid loop separately still, KINSOL can't handle that no matter what the state variables are 😞 |
|
@bryanwweber ... I believe KINSOL does support constraints, see one of the examples and you can put constraints on number of iterations, see https://sundials.readthedocs.io/en/latest/kinsol/Usage/index.html#c.KINSetNumMaxIters One big caveat is that I haven't spent any time with implementations here/ |
|
I think this is the wrong place for this general discussion on the relative merits of different approaches to solving the steady-state problem -- we already have an enhancement issue here: Cantera/enhancements#31. |
|
Looks like this PR got an ‘honorable’ mention here … |
Cantera can solve systems of equations representing physically zero-dimensional (i.e., time dependent-only) or one-dimensional (i.e., time + space) systems. The solution of the 1-D problems is done under the assumption of steady state, resulting in a differential-algebraic system of equations to solve. There is a solver implemented in Cantera as part of the 1-D code that solves these problems. 0-D systems can either be transient or steady state, represented by an ODE or algebraic equation respectively; at the moment, the only way to achieve solutions of the steady state problem is to integrate the transient problem until properties stop changing. We would like to modify or implement the existing steady-state 1-D solver for 0-D systems.
Changes proposed in this pull request
Newton, a simplified version of Cantera's 1-D solver for the solution of 0-D steady-state systems.solveSteady()functionality toReactorNetin C++ andsolve_steady()in Python.DenseMatrixfor optimized Jacobian computation.If applicable, fill in the issue number this pull request is fixing
Fixes #
Checklist
scons build&scons test) and unit tests address code coverage