Optimizations to Discontinuity Detection Scheme#3720
Conversation
Shreyas-Ekanathan
force-pushed
the
disco-optimizations
branch
from
931262d to
c11227a
Compare
| is_disco_step::Bool | ||
| disco_checkpoint::tType |
There was a problem hiding this comment.
We want to rename both of these.
There was a problem hiding this comment.
what would you name them?
| function set_discontinuity(integrator) | ||
| breakpointθ = find_discontinuity(integrator) | ||
| dt = integrator.dt | ||
| if 1.0e-10 < breakpointθ < 1.0 |
There was a problem hiding this comment.
pretty arbitrary, just don't want to take a tiny step (I think this is reasonable?)
There was a problem hiding this comment.
I think that's both smaller than we want (I think we probably never want and the wrong result if we hit it. I think it does make sense to clamp breakpointθ to (0.1, 0.9) because if we are on the wrong side of a disco point very close to the edge, we don't shrink the interval, but I think we still want to use disco if we have a sign crossing that we think is very close to an edge.
| end | ||
| disco_zero.ind = j | ||
| sol = solve(disco_prob) | ||
| sol = solve(disco_prob, tspan = (0.0, breakpointθ == -one(dt) ? 1.0 : breakpointθ); save_everystep = false) |
There was a problem hiding this comment.
does save_everystep do anything here? I didn't think any steps exist for NonlinearProblems. Also I wonder if using breakpointθ=-1 as the default makes sense. Could we use 1 to mean "no restriction"?
There was a problem hiding this comment.
I think using 1 is a little dicey since it potentially creates some numerical issues? whereas -1 is a cleaner sentinel imo.
save_everystep does prevent the solve from tracking every internal step I believe, so it should help?
There was a problem hiding this comment.
The argument for 1 is that it deosn't need to be a sentinel. 1.0 naturally means no restriction on the timestep. I don't think we track internal steps for a nonlinear solve.
| is_disco = integrator.is_disco_step | ||
| if is_disco | ||
| integrator.is_disco_step = false | ||
| cache.nconsteps = 0 |
There was a problem hiding this comment.
because it's not a standard timestep BDF shouldn't really count this step towards its order selection logic right?
There was a problem hiding this comment.
@ChrisRackauckas thoughts on this? I don't know how we want disco to interact with BDF well enough here.
2 participants
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Optimizations:
tstopon the first identification of a discontinuity to help with step-resizing and prevent excessively large steps after finding a discontinuity. This should help us not take unreasonable steps, as Oscar suggested, and helps with error resetting after passing a discontinuity (since we are effectively in a new regime) to begin stepping anew._ode_addsteps!out of the zero_func_struct call and only call it once per discontinuity detection run