Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quasi-Newton update rules issues #382

Open
2 of 4 tasks
mateuszbaran opened this issue Apr 22, 2024 · 3 comments
Open
2 of 4 tasks

Quasi-Newton update rules issues #382

mateuszbaran opened this issue Apr 22, 2024 · 3 comments

Comments

@mateuszbaran
Copy link
Member

mateuszbaran commented Apr 22, 2024

I'm currently working on some updates to quasi-Newton direction update rules. Here are a few things to check/consider:

  • QuasiNewtonCautiousDirectionUpdate doesn't seem to use θ function anywhere.
  • QuasiNewtonLimitedMemoryDirectionUpdate doesn't seem to use scale.
  • Some direction updates need a reset mechanism called by initialize_solver!, and maybe also when the direction stops being a descent direction. That shouldn't happen for a well-designed set of options but it's a bit hard to tell which ones are safe from that issue.
  • Unify projections to a custom function instead of a boolean flag.

I'm also working on some variant of direction update for manifolds with corners inspired by L-BFGS-B. There are different approaches with elaborate line searches and update matrices but I'm aiming for something relatively simple and generic. I'm not aiming at competing with Fortran codes, my goal is making combined manifold-box constrained optimization work relatively well.

@kellertuer
Copy link
Member

I started looking a bit into these points.

Ad 1) The function is used when updating the Hessian at

bound = d.θ(norm(M, p, X))

or
bound = d.θ(norm(M, p_old, get_gradient(mp, p_old)))

Ad 2) That might indeed be the case, I could only find the scale in the update of the full-matrix variants.

if iter == 1 && d.scale == true
d.matrix = skyk_c / inner(M, p, st.sk, st.sk) * d.matrix
end

I am not sure where the scaling was “lost” in one of the reworks, but we can surely bring that back

Ad 3) Sure, I am not 100% sure how to realise that, but when we have the iterate available doing a reset when that is zero sounds reasonable. When we wrote this code, reusing the state (and hence the updates) was not so much thought given to, for sure.

  • I also noticed that one of the recent updates that meant to introduce a stability idea did

project::Bool=true,

and
d.project && embed_project!(M, r, p, r)

which we basically have used before but then a bit more generic (not just a boolean)

* `project!`: (`copyto!`) for numerical stability it is possible to project onto

and then
tcgs.project!(M, tcgs.δ, p, tcgs.δ)

which might be a bit nicer to allow also for other ways to care for stability.
We should both unify this and probably also switch the second one to embed_project! (since that default is older than the embed/project discussion in Manifolds.jl).

@mateuszbaran
Copy link
Member Author

Ad 1) The function is used when updating the Hessian at

I see, thanks.

Ad 3) Sure, I am not 100% sure how to realise that, but when we have the iterate available doing a reset when that is zero sounds reasonable. When we wrote this code, reusing the state (and hence the updates) was not so much thought given to, for sure.

I have it sketched already so I will only ask for a review when it's ready 🙂

We should both unify this and probably also switch the second one to embed_project! (since that default is older than the embed/project discussion in Manifolds.jl).

Yes, unifying that is a good idea.

@kellertuer
Copy link
Member

If you see how to correctly add the scaling ( and unifying that to be a real value), feel free to add that as well. I am sure it was from one of the Wuang papers about QN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants