Add a nlls trait to BVProblem #567
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #567 +/- ##
==========================================
- Coverage 38.43% 30.03% -8.41%
==========================================
Files 55 55
Lines 4298 4309 +11
==========================================
- Hits 1652 1294 -358
- Misses 2646 3015 +369 ☔ View full report in Codecov by Sentry. |
avik-pal
force-pushed
the
ap/nlls_bvp
branch
from
dfdd353
to
12eab56
Compare
| @@ -115,6 +115,8 @@ struct ODESolution{T, N, uType, uType2, DType, tType, rateType, P, A, IType, S, | |||
| stats::S | |||
| alg_choice::AC | |||
| retcode::ReturnCode.T | |||
| resid::R | |||
| original::O | |||
There was a problem hiding this comment.
What about creating a BVPSolution to specifically handle these details only in the BVP context?
There was a problem hiding this comment.
I am open to that. We can do that by subtyping AbstractODESolution.
There was a problem hiding this comment.
IMO it just seems a bit wasteful code wise. It's just two more fields, and original makes sense in the IVP case as well (we should use it with ODEInterface)
avik-pal
force-pushed
the
ap/nlls_bvp
branch
from
4fbd019
to
e1e511a
Compare
src/solutions/ode_solutions.jl
Outdated
| struct BVPSolution{T, N, uType, uType2, DType, tType, rateType, P, A, IType, S, | ||
| AC <: Union{Nothing, Vector{Int}}, R, OI, ON} <: | ||
| AbstractODESolution{T, N, uType} | ||
| u::uType | ||
| u_analytic::uType2 | ||
| errors::DType | ||
| t::tType | ||
| k::rateType | ||
| prob::P | ||
| alg::A | ||
| interp::IType | ||
| dense::Bool | ||
| tslocation::Int | ||
| stats::S | ||
| alg_choice::AC | ||
| retcode::ReturnCode.T | ||
| resid::R | ||
| original_ivp::OI | ||
| original_nlsolve::ON | ||
| end |
There was a problem hiding this comment.
@ErikQQY thoughts on this?
There was a problem hiding this comment.
👍I think with this we can specifically handle solutions in the BVP context
avik-pal
force-pushed
the
ap/nlls_bvp
branch
from
e1e511a
to
344f505
Compare
Co-authored-by: Qingyu Qu <52615090+ErikQQY@users.noreply.github.com>
avik-pal
force-pushed
the
ap/nlls_bvp
branch
2 times, most recently
from
1dd64ac
to
21dbeea
Compare
avik-pal
changed the title
|
I am happy with the current state of this. To summarize
|
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
The main compilation cost for BVDiffEq is coming from solving NLLS problems. Having this allows us to have a type stable path for NLLS BVPs.