new stepsize rule adaPG(pi,r) #31
Conversation
| end | ||
|
|
||
| function OurRulePlus(; gamma = 0, nu = 1, xi = 1, r = 1/2) | ||
| _gamma = if gamma > 0 |
There was a problem hiding this comment.
You could also avoid introducing _gamma and just do
if gamma <= 0
error(…)
end
no?
There was a problem hiding this comment.
I see. This stepsize rule is fed into the PD algorithm. That is why I kept the same style as we had for OurRule. Only sigma is changed to gamma because we should first check how exactly the stepsize rule looks like there.
| r::R | ||
| end | ||
|
|
||
| function OurRulePlus(; gamma = 0, nu = 1, xi = 1, r = 1/2) |
There was a problem hiding this comment.
Why use the default gamma = 0? I guess it’s required positive to initialize the stepsize sequence, so you can also not give it a default
There was a problem hiding this comment.
Do we keep it in the same style as OurRule (to be updated with added sigma for the primal-dual version), or shall I remove gamma already?
|
|
||
| function stepsize(rule::OurRulePlus) | ||
| gamma = rule.gamma | ||
| return (gamma, gamma), (gamma, gamma) |
There was a problem hiding this comment.
I think the convention we used elsewhere is for the second stepsize returned to be the dual stepsize (after fixing the ratio between the two). So here the ratio is just 1 right?
There was a problem hiding this comment.
Then again this is because in the near future we should add the primal-dual stepsize version with first (gamma, gamma) transforming into (gamma, sigma).
| @@ -272,6 +272,43 @@ function stepsize(rule::OurRule, (gamma1, gamma0), x1, grad_x1, x0, grad_x0) | |||
| return (gamma, sigma), (gamma, gamma1) | |||
| end | |||
|
|
|||
|
|
|||
|
|
|||
| struct OurRulePlus{R} | |||
There was a problem hiding this comment.
Man, we really need to improve the naming convention 😅
Should we just use “author initials + year + rule” all over the module?
There was a problem hiding this comment.
Maybe in a separate PR though, I can do that
There was a problem hiding this comment.
I agree it is a mess. The name OurRulePlus is terrible indeed. Your suggestion sounds like a great idea.
I have added a new stepsize rule according to AdaPG method presented in https://arxiv.org/pdf/2311.18431.pdf