Implement Discontinuity Detection Algorithm#3121
Conversation
|
|
|
@ChrisRackauckas @oscardssmith If you could review when you get a chance, that would be great. Test failures seem to be a strange error independent of this. |
|
The CI machine maybe ran out of disk space? |
|
The test failure looks real. |
I'm not really sure what is failing here (e.g. failure in AD test), |
| Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" | ||
| EnzymeCore = "f151be2c-9106-41f4-ab19-57ee4f262869" | ||
| SciMLOperators = "c0aeaf25-5076-4817-a8d5-81caf7dfa961" | ||
| SciMLStructures = "53ae85a6-f571-4167-b2af-e1d143709226" | ||
| ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b" | ||
| Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697" | ||
| Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e" | ||
| ArrayInterface = "4fba245c-0d91-5ea0-9b3e-6abc04ee57a9" | ||
| ConcreteStructs = "2569d6c7-a4a2-43d3-a901-331e8e4be471" | ||
| FunctionWrappersWrappers = "77dc65aa-8811-40c2-897b-53d922fa7daf" | ||
| DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8" | ||
| DiffEqBase = "2b5f629d-d688-5b77-993f-72d75c75574e" | ||
| DocStringExtensions = "ffbed154-4ef7-542d-bbb7-c09d3a79fcae" | ||
| EnumX = "4e289a0a-7415-4d19-859d-a7e5c4648b56" |
There was a problem hiding this comment.
I added FunctionWrappers to core and I think it shuffled things around
| determine_controller_datatype(u::AbstractVector{<:Number}, internalnorm, ts::Tuple{<:Integer, <:Integer}) = promote_type(typeof(DiffEqBase.value(internalnorm(u, ts[1]))), typeof(DiffEqBase.value(internalnorm(u, ts[2]))), eltype(float.(DiffEqBase.value(ts)))) | ||
| determine_controller_datatype(u, internalnorm, ts::Tuple{<:Integer, <:Integer}) = promote_type(typeof(float(DiffEqBase.value(ts[1]))), typeof(float(DiffEqBase.value(ts[2])))) # This seems to be an assumption implicitly taken somewhere | ||
|
|
||
| struct DiscoProblem{ZF, NLP} |
There was a problem hiding this comment.
why do we need this? doesn't the nlp hold the zerofunc?
There was a problem hiding this comment.
I was working with Claude and it suggested that the extraction of the zerofunc from the NLP might create some of the type instability we were seeing so I tried this out and it seemed to help from a performance perspective. I removed functionwrappers for the same reason because it was hurting the type stability
There was a problem hiding this comment.
that's voodoo, it wouldn't effect type-stability. Where's the actual inference lost?
There was a problem hiding this comment.
so the instability happens during the updates to disco_zero in find_discontinuity, the root cause appears to be the Vector{IntervalNonLinearProblem} which leaves us no info about the zero_func_struct contained inside each function and creates the inference loss.
There was a problem hiding this comment.
so the previous fix would actually help something I think, becuase when you put everything into that custom wrapper that stores the information about the zero_func_struct, we maintain that type info and so the updates become stable
There was a problem hiding this comment.
A functionwrapper is effectively a type with that information 😅 what's the issue you're hitting with that?
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
A preliminary implementation of autonomous discontinuity detection in Radau. Uses a PR to SciML base at SciML/SciMLBase.jl#1219 to modify callbacks to support a is_discontinuous flag.