Including jvp for radius update schemes #172
Conversation
src/jacobian.jl
Outdated
| @@ -144,3 +144,14 @@ function jacobian_autodiff(f, x::AbstractArray, nonlinfun, alg) | |||
| jac_prototype = jac_prototype, chunksize = chunk_size), | |||
| num_of_chunks) | |||
| end | |||
|
|
|||
| function jvp(cache::TrustRegionCache{false}) | |||
There was a problem hiding this comment.
Put a ! on there because it's mutating.
There was a problem hiding this comment.
we are actually not modifying the cache, we are just returning the jvp. Should that still be done?
There was a problem hiding this comment.
You are modifying the cache in one of them
src/trustRegion.jl
Outdated
|
|
||
|
|
||
| @unpack p1= cache | ||
| cache.trust_r = p1 * cache.internalnorm(jvp(cache)) # we need the gradient at the new (k+1)th point WILL THIS BECOME ALLOCATING? |
There was a problem hiding this comment.
The comment can be removed.
|
Needs to add the dependency. |
Codecov Report
@@ Coverage Diff @@
## master #172 +/- ##
==========================================
+ Coverage 85.08% 91.88% +6.80%
==========================================
Files 7 7
Lines 590 604 +14
==========================================
+ Hits 502 555 +53
+ Misses 88 49 -39
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/NonlinearSolve.jl
Outdated
| @@ -61,6 +61,7 @@ SnoopPrecompile.@precompile_all_calls begin for T in (Float32, Float64) | |||
| end end | |||
|
|
|||
| export RadiusUpdateSchemes | |||
| export auto_jacvec, auto_jacvec! | |||
There was a problem hiding this comment.
| export auto_jacvec, auto_jacvec! |
| @@ -441,7 +445,7 @@ function dogleg!(cache::TrustRegionCache) | |||
|
|
|||
| # Test if the full step is within the trust region. | |||
| if norm(u_tmp) ≤ trust_r | |||
| cache.step_size = u_tmp | |||
| cache.step_size = deepcopy(u_tmp) | |||
There was a problem hiding this comment.
Is this supposed to be a .=?
|
The issue is resolved, if there's nothing else, i think we can merge this pr. |
|
Test failed, looks like a tolerance issue. |
|
works now |
No description provided.