Handle edge-cases of inverted root pairs in the condition rootfinding

[DaniGlez]

Closes #1290 (see the issue discussion for context)

Checklist

• Appropriate tests were added (requires Make ModAB iteration always shrink the bracket even if there's numerical noise NonlinearSolve.jl#861 to be merged)
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

Add any other context about the problem here.

[DaniGlez]

Also please squash if merging, I was a bit sloppy with the WIP commits 😅

[ChrisRackauckas]

Needs a test

[DaniGlez]

Test added; I would still like SciML/NonlinearSolve.jl#861 to be merged first, so that the ModAB behavior is consistent (if it hits the buggy path, a slightly different original root might be found)

[DaniGlez]

Fuzzer script for posterity, in case the test needs to be recreated in the future: https://gist.github.com/DaniGlez/6100f2530fc76d4f9673a8f9f7422564

[DaniGlez]

I think the JumpProcesses test failure should be solved by SciML/NonlinearSolve.jl#861 as well

[ChrisRackauckas]

It seems like the bracketing methods should just be made better for this, rather than hacking it in here?

[DaniGlez]

I'd be OK with that. The reason I made it here is because, in the context of a general bracketing problem, if there are multiple numerical roots (ie the condition switches signs multiple times in a small interval, or e.g. if there are multiple zeros in a row) it's not clear which one of the numerical root pairs should be preferred by the solver, whereas in this context we want the first one. One option would be to implement the optional functionality in BracketingNonlinearSolve and just call it from here; would that be acceptable?

[ChrisRackauckas]

it's not clear which one of the numerical root pairs should be preferred by the solver, whereas in this context we want the first one. One option would be to implement the optional functionality in BracketingNonlinearSolve and just call it from here; would that be acceptable?

Yes. @oscardssmith wanted to do a thing where we made it a whole thing: make a brackating problem for it, and develop solvers specifically for it. We'd get a lot more performance out by really specializing this problem more, and it has many other use cases beyond the callback handling. So, taking it to nonlinearsolve in general seems like the right direction.

But at least the trick here, it seems like the solver just gets stuck because of a numerical error, and this just flips the brackets and tells it to solve again. To me, it seems like we should just make sure the solvers don't get stuck, and if their loop needs to do this in order to be accurate, it should get fixed there.

So I'll close given the assumption it's upstreaming.