Add SciML.build_solution #28
Conversation
|
yeah it's already doing it to exit, so it might as well use the calculation of the exit criterion. |
src/solve.jl
Outdated
| @@ -2,14 +2,8 @@ function SciMLBase.solve(prob::NonlinearProblem, | |||
| alg::AbstractNonlinearSolveAlgorithm, args...; | |||
| kwargs...) | |||
| solver = init(prob, alg, args...; kwargs...) | |||
| sol = solve!(solver) | |||
| sol, resid = solve!(solver) | |||
There was a problem hiding this comment.
solve! has a standard interface that shouldn't be broken. We should just stick the resid in the sol.
There was a problem hiding this comment.
Okay, I will update the sol struct with resid.
src/solve.jl
Outdated
| sol = solve!(solver) | ||
| return sol | ||
| sol, resid = solve!(solver) | ||
| if typeof(sol) <: NewtonSolution |
There was a problem hiding this comment.
Why is this branching needed? It makes the solution type unstable and break non-Newton solvers. We can just assume that sol.u exists.
There was a problem hiding this comment.
There are two types of solver, Newton solver and Bracketing methods, which outputs sol.left and sol.right. That's why I wanted to ask how to pass bracketing methods to build_solution?
src/scalar.jl
Outdated
| @@ -75,14 +76,14 @@ function SciMLBase.solve(prob::NonlinearProblem, ::Bisection, args...; maxiters | |||
| fl, fr = f(left), f(right) | |||
|
|
|||
| if iszero(fl) | |||
| return BracketingSolution(left, right, EXACT_SOLUTION_LEFT) | |||
| return BracketingSolution(left, right, EXACT_SOLUTION_LEFT,fl) | |||
There was a problem hiding this comment.
Do I need to return SciMLBase.build_solution here too, (scalar.jl)? Because if we need to do, we need to check how u0 as a Tuple will be handled here: https://github.com/SciML/SciMLBase.jl/blob/master/src/solutions/nonlinear_solutions.jl#L26? What should it be in that case? @ChrisRackauckas
There was a problem hiding this comment.
Maybe change it to N = length((size(u)...,)) since it should not be able to change over the solution. Yes, it should use build_solution like all others, and treat u as the left.
There was a problem hiding this comment.
or well, based on EXACT_SOLUTION_LEFT choose u as left etc.
|
With SciML/SciMLBase.jl#17, this will complete the PR. |
src/types.jl
Outdated
| end | ||
|
|
||
| # function NewtonImmutableSolver{iip}(iter, f, alg, u, fu, p, force_stop, maxiters, internalnorm, retcode, tol, cache) where iip | ||
| # NewtonImmutableSolver{iip, typeof(f), typeof(alg), typeof(u), | ||
| # typeof(fu), typeof(p), typeof(internalnorm), typeof(tol), typeof(cache)}(iter, f, alg, u, fu, p, force_stop, maxiters, internalnorm, retcode, tol, cache) | ||
| # end | ||
|
|
||
| struct BracketingSolution{uType} | ||
| struct BracketingSolution{uType,resType} |
There was a problem hiding this comment.
Are we still using the original solution type?
There was a problem hiding this comment.
Nope, we don't need it. Refactored the code. Thanks for the suggestion.
TrustRegion -> SimpleTrustRegion and specialize the number types
Hi,
I have currently added
build_solutionin solve.jl for NewtonRaphson. I did understand what was the purpose ofscalar.jl. Do I need to addbuild_solutionthere too? And how to handle bracketing methods forbuild_solution?@ChrisRackauckas, please have a look.