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

Adaptive Regularisation with Cubics (ARC) Solver #277

Merged
merged 75 commits into from
Aug 23, 2023
Merged

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Jul 5, 2023

This PR is a slight rework from the work @mathiasrm1 did in his master thesis this semester. Until now

  • Variables are renamed according to other solvers
  • some temporary variables were introduced to avoid allocations
  • Lanczos iteration optimised a bit
  • The Cubic subproblem cost and gradient implemented
  • add middle-layer solver interface adaptive_regularization_with_cubics(M, mho, ...; kwargs...)
  • allow / test a CG subsolver as well (cost and grad already existing
  • write a documentation page
  • write a tutorial comparing trust region and ARC-Lanczos, and ARC-CG
  • check the value theta (maybe in a Newton State?)
  • make Newton a state so it can have proper stopping criteria.
  • there seems to also be a problem with the second order progress stopping criterion as to when to stop Lanczos. It currently often uses its maximal iterations. The only reason the outer one did not hit, was that it checked versus one too less.
  • write tests
  • check that the subproblem can be solved with CG on the tangent space.
  • write a status_summary for the new state.
  • improve naming and availability of the sub solver stopping criteria
  • carefully check rendered docs
  • maybe implement the exact best step size for the gradient descent subsolver

mathiasrm1 and others added 30 commits January 31, 2023 09:08
# Conflicts:
#	src/Manopt.jl
#	src/plans/stopping_criterion.jl
#	src/solvers/gradient_descent.jl
@kellertuer
Copy link
Member Author

kellertuer commented Aug 8, 2023

🥳finally found my last few bugs such that now also GradientDescent and CG can be used as sub solvers – now it just remains to

  • write tests
  • improve stopping criterion names
  • finish the accompanying ManoptExamples tutorial – this is done but of course has to wait until this is merged and published.

@kellertuer kellertuer marked this pull request as ready for review August 9, 2023 17:59
@kellertuer kellertuer removed the WIP Work in Progress (for a pull request) label Aug 12, 2023
@kellertuer
Copy link
Member Author

The only thing to maybe consider is, that for the gradient based sub solvers of ARC, there is
https://github.com/NicolasBoumal/manopt/blob/master/manopt/solvers/arc/solve_along_line.m
which can be considered an exact step size computation. But it is not documented where that is from nor why that holds, so it is maybe not so easy to generalise / transfer here as a step size.
The only effect for now is, that here the gradient based methods require a few more steps than the Lanczos solver.

@kellertuer kellertuer added the Ready-for-Review A label for pull requests that are feature-ready label Aug 12, 2023
@kellertuer
Copy link
Member Author

Currently code coverage fails because we have one spurious line missed on code not changed here. All lines coded here are covered by tests.

# Conflicts:
#	src/Manopt.jl
#	src/plans/subsolver_plan.jl
#	test/plans/test_debug.jl
… from 0.4.31. Remove ARC example since that is more extended in the ManoptExamples.
@kellertuer
Copy link
Member Author

Let's leave the optimised step size for a later PR.

@kellertuer kellertuer merged commit 0ab53d6 into master Aug 23, 2023
12 of 13 checks passed
@kellertuer kellertuer deleted the mathiasrm1/arc branch November 20, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ready-for-Review A label for pull requests that are feature-ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants