Fix tests failings for TwoPointBVPFunction #517
Conversation
Signed-off-by: ErikQQY <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #517 +/- ##
==========================================
+ Coverage 53.37% 56.30% +2.93%
==========================================
Files 50 50
Lines 3798 3806 +8
==========================================
+ Hits 2027 2143 +116
+ Misses 1771 1663 -108
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -119,25 +119,25 @@ struct BVProblem{uType, tType, isinplace, P, F, BF, PT, K} <: | |||
| else | |||
| @assert prob_type === problem_type "This indicates incorrect problem type specification! Users should never pass in `problem_type` kwarg, this exists exclusively for internal use." | |||
| end | |||
| return new{typeof(u0), typeof(_tspan), iip, typeof(p), typeof(f), typeof(bc), | |||
| typeof(problem_type), typeof(kwargs)}(f, bc, u0, _tspan, p, problem_type, | |||
| return new{typeof(u0), typeof(_tspan), iip, typeof(p), typeof(f), typeof(f.bc), | |||
There was a problem hiding this comment.
Why is the bc out here at all? Isn't it in the BVPFunction?
There was a problem hiding this comment.
@avik-pal is it actually needed?
There was a problem hiding this comment.
The bc was kept outside similar to the SDEProblem interface carrying around the g outside (I think?)
There was a problem hiding this comment.
That was just fixed in the v2.0. I thought this was fixed in the v2.0 as well 😅. Fix and slip it in ASAP.
There was a problem hiding this comment.
The modifications in BVProblem is similiar to SDEProblem, just try to make them consistent
There was a problem hiding this comment.
SDEProblem is still carrying around g
SciMLBase.jl/src/problems/sde_problems.jl
Lines 88 to 89 in 75b1925
There was a problem hiding this comment.
Yeah, SDEProblem is still carrying g, so in this way I think we can still use prob.g to directly access g
There was a problem hiding this comment.
Do we need to remove g in the problem struct?
|
This PR is now out of date because of the changes in #518, so closing, but a similar PR to the new function form is required. |
This should fix the tests failings in DiffEqBase.jl SciML/DiffEqBase.jl#940
@ChrisRackauckas @avik-pal