Implementing some new Trust region radius update schemes #159

yash2798 · 2023-03-06T01:13:55Z

No description provided.

yash2798 · 2023-03-06T01:19:04Z

@ChrisRackauckas is this the correct way to implement this? here, we take the input from the user regarding what update scheme they would like to use as a String solve(probN, TrustRegion(radius_update_scheme="<>"), abstol = 1e-9). and then use this input subsequently?

ChrisRackauckas · 2023-03-06T01:23:46Z

we take the input from the user regarding what update scheme they would like to use as a String solve(probN, TrustRegion(radius_update_scheme="<>"), abstol = 1e-9). and then use this input subsequently?

No, never take in a string. Take in a type instance, like the algorithm choice.

yash2798 · 2023-03-06T01:32:01Z

we take the input from the user regarding what update scheme they would like to use as a String solve(probN, TrustRegion(radius_update_scheme="<>"), abstol = 1e-9). and then use this input subsequently?

No, never take in a string. Take in a type instance, like the algorithm choice.

okay, got it, so I define all the radius update schemes as composite types? and then use them? just like the TrustRegion type is defined?

yash2798 · 2023-03-07T01:59:33Z

Hi @ChrisRackauckas , I made some changes regarding how we choose the relevant radius update scheme. Is this approach alright?

codecov · 2023-03-07T15:32:53Z

Codecov Report

Merging #159 (b1f34dd) into master (f32e596) will decrease coverage by 7.06%.
The diff coverage is 36.50%.

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
- Coverage   91.28%   84.22%   -7.06%     
==========================================
  Files           7        7              
  Lines         505      558      +53     
==========================================
+ Hits          461      470       +9     
- Misses         44       88      +44

Impacted Files	Coverage Δ
src/NonlinearSolve.jl	`60.00% <ø> (ø)`
src/utils.jl	`62.31% <0.00%> (-3.84%)`	⬇️
src/trustRegion.jl	`76.53% <38.98%> (-21.93%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ChrisRackauckas · 2023-03-07T17:18:27Z

src/trustRegion.jl

+struct RadiusUpdate{B}
+    simple::B
+    hei::B
+    yuan::B
+    bastin::B


Suggested change

struct RadiusUpdate{B}

simple::B

hei::B

yuan::B

bastin::B

struct RadiusUpdate

simple::Bool

hei::Bool

yuan::Bool

bastin::Bool

ChrisRackauckas · 2023-03-07T17:18:43Z

src/trustRegion.jl

+      return RadiusUpdate{Bool}(true, false, false, false)
+    elseif hei
+      return RadiusUpdate{Bool}(false, true, false, false)
+    elseif yuan
+      return RadiusUpdate{Bool}(false, false, true, false)
+    elseif bastin
+      return RadiusUpdate{Bool}(false, false, false, true)


Suggested change

return RadiusUpdate{Bool}(true, false, false, false)

elseif hei

return RadiusUpdate{Bool}(false, true, false, false)

elseif yuan

return RadiusUpdate{Bool}(false, false, true, false)

elseif bastin

return RadiusUpdate{Bool}(false, false, false, true)

return RadiusUpdate{Bool}(true, false, false, false)

elseif hei

return RadiusUpdate{Bool}(false, true, false, false)

elseif yuan

return RadiusUpdate(false, false, true, false)

elseif bastin

return RadiusUpdate(false, false, false, true)

ChrisRackauckas · 2023-03-07T17:19:24Z

src/trustRegion.jl

+function RadiusUpdate(;simple::Bool = Val{true}(),  # 3 different radius update schemes
+                      hei::Bool = Val{false}(),
+                      yuan::Bool = Val{false}(),
+                      bastin::Bool = Val{false}())
+    if simple
+      return RadiusUpdate{Bool}(true, false, false, false)
+    elseif hei
+      return RadiusUpdate{Bool}(false, true, false, false)
+    elseif yuan


This doesn't make sense. If it's going to be non-dispatching logic, use an EnumX.

Oh yes, I am sorry I misinterpreted the use of Val, but now it's clear. I have changed the overall approach.

yash2798 · 2023-03-08T00:33:20Z

@ChrisRackauckas I think this approach would be better! It would be great if you can take a look.

I have a question though:
In line 103, the radius_update_scheme input is a Symbol. Another option is to use Val{:simple}() which will change the function call for trust_region_step! in lines 299 and 322. So, should the user input be kept as a Symbol (:simple, :hei, etc.) or should I change it to Val{:simple}(). I mean, there is not a lot of difference between the two but what method would be better from a user's pov?

yash2798 · 2023-03-08T00:35:19Z

and thanks for all the feedback that you have been giving, I am learning a lot :)

ChrisRackauckas · 2023-03-08T04:20:15Z

In line 103, the radius_update_scheme input is a Symbol. Another option is to use Val{:simple}() which will change the function call for trust_region_step! in lines 299 and 322. So, should the user input be kept as a Symbol (:simple, :hei, etc.) or should I change it to Val{:simple}(). I mean, there is not a lot of difference between the two but what method would be better from a user's pov?

Val{:sample}() is a valid construction, so that doesn't give good errors for typos.

See the ReturnCodes implementation and EnumX.jl

https://github.com/SciML/SciMLBase.jl/blob/master/src/retcodes.jl

yash2798 · 2023-03-09T00:39:51Z

Okay, so by this you mean that if the user makes a typo :sample instead of the required :simple, then the program will show an exception something like this no method matching trust_region_step!(cache::TrustRegionCache, ::Val{:sample}) and this error is not very clear for the user? Is this what you mean?

If yes, couldn't we simply check for the validity of the input beforehand and raise an exception in case of any mismatch?

yash2798 · 2023-03-09T02:25:21Z

@ChrisRackauckas Is this approach with EnumX alright? I have used my previous approach of creating a RadiusUpdate type variable and then used enumx just for multiple dispatching.

YingboMa · 2023-03-09T05:00:26Z

src/trustRegion.jl

+    Bastin
+end
+
+struct RadiusUpdate


Aren't they mutually exclusive? We should just use enum.

Yes, the structs can be deleted.

ChrisRackauckas · 2023-03-09T10:37:08Z

Everything should use the enums and the structs should be deleted.

ChrisRackauckas · 2023-03-10T12:12:27Z

src/trustRegion.jl

       AbstractNewtonAlgorithm{CS, AD, FDT, ST, CJ}
    linsolve::L
    precs::P
+    radius_update_scheme::RUS


Suggested change

radius_update_scheme::RUS

radius_update_scheme::RadiusUpdateSchemes.T

It doesn't need to be parametric.,

ChrisRackauckas · 2023-03-10T12:12:41Z

src/trustRegion.jl

@@ -138,6 +147,7 @@ mutable struct TrustRegionCache{iip, fType, algType, uType, resType, pType,
    retcode::SciMLBase.ReturnCode.T
    abstol::tolType
    prob::probType
+    radius_update_scheme::radType


ChrisRackauckas · 2023-03-10T12:13:40Z

src/trustRegion.jl

    return nothing
 end

-function trust_region_step!(cache::TrustRegionCache)
+function trust_region_step!(cache::TrustRegionCache, ::Val{0}) # conventional radius update scheme 


Make a single dispatch and branch

Suggested change

function trust_region_step!(cache::TrustRegionCache, ::Val{0}) # conventional radius update scheme

function trust_region_step!(cache::TrustRegionCache)

@unpack radius_update_scheme = cache

if radius_update_scheme == RadiusUpdateSchemes.Simple

#method 1

elseif radius_update_scheme == RadiusUpdateSchemes.Hei

#method 2

...

end

end

do you mean like this in a single method?

cool! also if I need some helper functions, like in Hei's scheme, there is a monotonic function used which depends on the actual:predicted change ratio. So these kind of helper functions can go in utils.jl, right?

ChrisRackauckas · 2023-03-10T12:13:52Z

src/NonlinearSolve.jl

@@ -10,6 +10,7 @@ using ForwardDiff: Dual
 using LinearAlgebra
 using StaticArraysCore
 using RecursiveArrayTools
+import EnumX


missing from the Project.toml

yeah i added the pkg in the env but didn't commit it yet. will do it in the next commit

src/trustRegion.jl

ChrisRackauckas · 2023-03-11T14:04:21Z

src/trustRegion.jl

+          cache.force_stop = true
+      end
+
+    elseif radius_update_scheme === RadiusUpdateSchemes.Hei


error for not implemented

okay, like a TypeError when the radius_update_scheme type doesn't match RadiusUpdateSchemes.T ?

ChrisRackauckas · 2023-03-15T07:24:40Z

src/trustRegion.jl

+    # Parameters for the Schemes
+    p1 = 0
+    p2 = 0
+    p3 = 0
+    p4 = 0
+    ϵ = 1e-8
+    if radius_update_scheme === RadiusUpdateSchemes.Hei
+      step_threshold = 0
+      shrink_threshold = 0.25
+      expand_threshold = 0.25
+      p1 = 5.0 # M
+      p2 = 0.1 # β
+      p3 = 0.15 # γ1
+      p4 = 0.15 # γ2
+    elseif radius_update_scheme === RadiusUpdateSchemes.Yuan
+      step_threshold = 0.0001
+      shrink_threshold = 0.25
+      expand_threshold = 0.25
+      p1 = 2.0 # μ
+      p2 = 1/6 # c5
+      p3 = 6 # c6
+      p4 = 0
+    end


these values should match the type of u0.

ChrisRackauckas · 2023-03-15T07:25:17Z

src/trustRegion.jl

+
+      # yuan's scheme
+      @unpack fu = cache
+      cache.trust_r = p1 * cache.internalnorm(jacobian(cache, f) * fu) # we need the gradient at the new (k+1)th point  WILL THIS BECOME ALLOCATING?


you don't needto calculate the whole Jacobian here, we can fix that in a future PR though.

ChrisRackauckas · 2023-03-18T03:52:02Z

Last step here is to make sure the tests don't fail, and fix the constants definitions so that they match the state types.

yash2798 · 2023-03-18T15:55:44Z

Last step here is to make sure the tests don't fail, and fix the constants definitions so that they match the state types.

Oh yes thanks, I understood it. I was just testing the two new methods myself and will make a commit real soon.

yash2798 · 2023-03-21T03:42:22Z

Hi @ChrisRackauckas I can't quite get how can we get the gradient in line 421 without the whole jacobian? Did you mean something through jvp?

Started implementing new radius update schemes

f9db2fe

yash2798 marked this pull request as draft March 6, 2023 02:01

yash-rs added 2 commits March 7, 2023 02:55

RadiusUpdate type instance

dc43d2d

some comments

2be3572

yash2798 marked this pull request as ready for review March 7, 2023 11:17

ChrisRackauckas reviewed Mar 7, 2023

View reviewed changes

modified radius scheme input method

a027372

modified approach using enumx

56b23b0

YingboMa reviewed Mar 9, 2023

View reviewed changes

yash-rs added 2 commits March 10, 2023 01:01

EnumX final

a256021

format

7d29b1a

ChrisRackauckas reviewed Mar 10, 2023

View reviewed changes

yash-rs added 2 commits March 11, 2023 00:43

EnumX pkg + finalise scheme input

7aa7e94

helper rfunc

764501e

ChrisRackauckas reviewed Mar 11, 2023

View reviewed changes

src/trustRegion.jl Outdated Show resolved Hide resolved

ChrisRackauckas reviewed Mar 11, 2023

View reviewed changes

src/trustRegion.jl Outdated Show resolved Hide resolved

ChrisRackauckas added 2 commits March 11, 2023 09:04

Update src/trustRegion.jl

503fc4c

Update src/trustRegion.jl

08fffd4

ChrisRackauckas reviewed Mar 11, 2023

View reviewed changes

yash-rs added 3 commits March 12, 2023 04:02

hei's method

76185a8

added hei's and yuan's schemes

4aa4813

some fixes

17abaf0

ChrisRackauckas reviewed Mar 15, 2023

View reviewed changes

type fixes

b1f34dd

ChrisRackauckas merged commit 0d07b27 into SciML:master Mar 21, 2023

yash2798 deleted the ys/tr_radius_schemes branch March 23, 2023 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing some new Trust region radius update schemes #159

Implementing some new Trust region radius update schemes #159

yash2798 commented Mar 6, 2023

yash2798 commented Mar 6, 2023

ChrisRackauckas commented Mar 6, 2023

yash2798 commented Mar 6, 2023

yash2798 commented Mar 7, 2023

codecov bot commented Mar 7, 2023 •

edited

Loading

ChrisRackauckas Mar 7, 2023

ChrisRackauckas Mar 7, 2023

ChrisRackauckas Mar 7, 2023

yash2798 Mar 8, 2023

yash2798 commented Mar 8, 2023

yash2798 commented Mar 8, 2023

ChrisRackauckas commented Mar 8, 2023

yash2798 commented Mar 9, 2023

yash2798 commented Mar 9, 2023

YingboMa Mar 9, 2023

ChrisRackauckas Mar 9, 2023

ChrisRackauckas commented Mar 9, 2023

ChrisRackauckas Mar 10, 2023

ChrisRackauckas Mar 10, 2023

ChrisRackauckas Mar 10, 2023

yash2798 Mar 10, 2023 •

edited

Loading

ChrisRackauckas Mar 10, 2023

yash2798 Mar 10, 2023

ChrisRackauckas Mar 10, 2023

yash2798 Mar 10, 2023

ChrisRackauckas Mar 11, 2023

yash2798 Mar 11, 2023

ChrisRackauckas Mar 15, 2023

ChrisRackauckas Mar 15, 2023

ChrisRackauckas commented Mar 18, 2023

yash2798 commented Mar 18, 2023

yash2798 commented Mar 21, 2023

	radius_update_scheme::RUS
	radius_update_scheme::RadiusUpdateSchemes.T

-function trust_region_step!(cache::TrustRegionCache, ::Val{0}) # conventional radius update scheme
+function trust_region_step!(cache::TrustRegionCache)
+        @unpack radius_update_scheme = cache
+        if radius_update_scheme == RadiusUpdateSchemes.Simple
+              #method 1
+        elseif radius_update_scheme == RadiusUpdateSchemes.Hei
+              #method 2
+        ...
+        end
+end

Implementing some new Trust region radius update schemes #159

Implementing some new Trust region radius update schemes #159

Conversation

yash2798 commented Mar 6, 2023

yash2798 commented Mar 6, 2023

ChrisRackauckas commented Mar 6, 2023

yash2798 commented Mar 6, 2023

yash2798 commented Mar 7, 2023

codecov bot commented Mar 7, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash2798 commented Mar 8, 2023

yash2798 commented Mar 8, 2023

ChrisRackauckas commented Mar 8, 2023

yash2798 commented Mar 9, 2023

yash2798 commented Mar 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisRackauckas commented Mar 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash2798 Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisRackauckas commented Mar 18, 2023

yash2798 commented Mar 18, 2023

yash2798 commented Mar 21, 2023

codecov bot commented Mar 7, 2023 •

edited

Loading

yash2798 Mar 10, 2023 •

edited

Loading