-
-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix implementation of advance_to_steady_state #654
Comments
It's now apparent this could use some work. As Referenced by @12Chao in #1305 (comment) the answer may be to revive the 0D solver in #1021, but @ischoegl could you elaborate a little on how one could use #629 to address this? |
The 0D solver in #1021 and solver derivatives obtained from #629 are orthogonal approaches. What I had in mind here was to use a convergence criterion based on vanishing derivatives (above ignition) rather than the 'feature scaling' (where I was never able to locate documentation that was sufficiently clear about the background). PS: I don't think that this would help with the integration failures in #1305 (comment) though ... |
Could the |
in general, I believe so. But it’s a very different approach. |
@12Chao As @ischoegl said, #1021 and On the other hand, Hope that helps! |
Thanks for the explanation, that's really helpful! |
Cantera version
all
Operating System
all
Python/MATLAB version
all
Expected Behavior
The choice of the Feature Scaling approach in
advance_to_steady_state
to determine convergence should use an actual time derivative.Actual Behavior
In the current implementation, convergence is checked for an absolute difference between integrator states. The step size is, however, internally chosen by CVODE and thus neither constant nor defined. As a consequence, convergence is poorly defined in the current code, as it depends on the CVODE integrator state as much as it does on
residual_threshold
.PR #629 introduces an interface to time derivatives calculated by CVODE, which can be used to address this issue.
The text was updated successfully, but these errors were encountered: