Skip to content

Commit 0e92b74

Browse files
Fix some issues with CG and certain coefficient rules when using restart (#604)
* refactor CGRule * Fix some issues with CG + HZ * provide a proper test + formatting * fix CG test * Refactor CG rules. * Fix 2 cases where we implicitly relied on storage being updated in case it was not filled and now fill it explicitly. * bump version. --------- Co-authored-by: Ronny Bergmann <git@ronnybergmann.net>
1 parent c6580a8 commit 0e92b74

7 files changed

Lines changed: 225 additions & 140 deletions

File tree

Changelog.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ The file was started with Version `0.4`.
66
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
77
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
88

9+
## [0.5.37] May 5, 2026
10+
11+
### Changed
12+
13+
* The default restart rule for `conjugate_gradient_descent` is now `RestartOnNonDescent` instead of `NeverRestart`, which makes the algorithm more robust to non-convexity and numerical issues. The old default can still be used by explicitly passing `restart_condition=NeverRestart()`. (#604)
14+
* `HagerZhangCoefficientRule` now has a safeguard against the denominator being too close to zero (the `denom_threshold` field). By default it is set to 1.0e-10. You can set it to a lower positive value (or even zero) to weaken the safeguard, but it is recommended to keep it to avoid numerical issues. (#604)
15+
* introduce for all `Rule`s also a variant without being encapsulated in a memory, where the old values have to be passed as keywords. This is now used by the `ConjugateGradientBealeRestartRule` when evaluating its inner rule. (#604)
16+
917
## [0.5.36] April 24, 2026
1018

1119
### Added

Project.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name = "Manopt"
22
uuid = "0fc0a36d-df90-57f3-8f93-d78a9fc72bb5"
3-
version = "0.5.36"
3+
version = "0.5.37"
44
authors = [{family-names = "Bergmann", given-names = "Ronny", alias = "kellertuer", city = "Trondheim", affiliation = "Norwegian University of Science and Technology", country = "NO", email = "manopt@ronnybergmann.net", orcid = "https://orcid.org/0000-0001-8342-7218", website = "https://ronnybergmann.net"}]
55

66
[workspace]

src/plans/conjugate_gradient_plan.jl

Lines changed: 179 additions & 129 deletions
Large diffs are not rendered by default.

src/plans/solver_state.jl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -500,10 +500,11 @@ end
500500
get_storage(a::AbstractStateAction, key::Symbol)
501501
502502
Return the internal value of the [`AbstractStateAction`](@ref) `a` at the
503-
`Symbol` `key`.
503+
`Symbol` `key`. Returns `nothing` if the key does not exist
504504
"""
505-
get_storage(a::AbstractStateAction, key::Symbol) = a.values[key]
506-
505+
function get_storage(a::AbstractStateAction, key::Symbol)
506+
return get(a.values, key, nothing)
507+
end
507508
"""
508509
get_storage(a::AbstractStateAction, ::PointStorageKey{key}) where {key}
509510

src/solvers/conjugate_gradient_descent.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ function conjugate_gradient_descent!(
142142
mgo::O,
143143
p;
144144
coefficient::Union{DirectionUpdateRule, ManifoldDefaultsFactory} = ConjugateDescentCoefficient(),
145-
restart_condition::AbstractRestartCondition = NeverRestart(),
145+
restart_condition::AbstractRestartCondition = RestartOnNonDescent(),
146146
retraction_method::AbstractRetractionMethod = default_retraction_method(M, typeof(p)),
147147
stepsize::Union{Stepsize, ManifoldDefaultsFactory} = default_stepsize(
148148
M, ConjugateGradientDescentState; retraction_method = retraction_method
@@ -195,7 +195,8 @@ function step_solver!(amp::AbstractManoptProblem, cgs::ConjugateGradientDescentS
195195
cgs.δ .-= cgs.X
196196
if (cgs.restart_condition(amp, cgs, k))
197197
# restart solver; set dir to -grad
198-
cgs.δ = -copy(get_manifold(amp), cgs.p, cgs.X)
198+
copyto!(M, cgs.δ, cgs.X)
199+
cgs.δ .*= -1
199200
update_storage!(cgs.coefficient.storage, amp, cgs)
200201
cgs.β = 0.0
201202
end

test/plans/test_conjugate_gradient_plan.jl

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
using Manopt, Manifolds, Test
22

33
struct DummyCGCoeff <: DirectionUpdateRule end
4-
(u::DummyCGCoeff)(p, s, k) = 0.2
4+
(::DummyCGCoeff)(pr, st, k; kwagrs...) = 0.2
5+
(::Manopt.DirectionUpdateRuleStorage{DummyCGCoeff})(pr, st, k) = 0.2
56
Manopt.update_rule_storage_points(::DummyCGCoeff) = Tuple{}
67
Manopt.update_rule_storage_vectors(::DummyCGCoeff) = Tuple{}
78

@@ -16,22 +17,26 @@ Manopt.update_rule_storage_vectors(::DummyCGCoeff) = Tuple{}
1617
p0 = [1.0, 0.0]
1718
pr = DefaultManoptProblem(M, ManifoldGradientObjective(f, grad_f))
1819
cgs2 = ConjugateGradientDescentState(
19-
M;
20-
p = p0,
20+
M; p = p0,
2121
stopping_criterion = StopAfterIteration(2),
2222
stepsize = Manopt.ConstantStepsize(M, 1.0),
2323
coefficient = dur2,
2424
)
2525
cgs2.X = [0.0, 0.2]
26+
# Fake update history to get a certain old X and old p
27+
cgs2.coefficient(pr, cgs2, 0)
28+
# the inner check is 0.2 which is still less than 0.3
2629
@test cgs2.coefficient(pr, cgs2, 1) != 0
2730
cgs3 = ConjugateGradientDescentState(
28-
M;
29-
p = p0,
31+
M; p = p0,
3032
stopping_criterion = StopAfterIteration(2),
3133
stepsize = Manopt.ConstantStepsize(M, 1.0),
3234
coefficient = dur3,
3335
)
3436
cgs3.X = [0.0, 0.2]
37+
# Fake update history to get a certain old X and old p
38+
cgs3.coefficient(pr, cgs3, 0)
39+
# then we are above the threshold 0.1 (namely at 0.2) and we get a descent step
3540
@test cgs3.coefficient(pr, cgs3, 1) == 0
3641
end
3742
@testset "representation and summary of Coefficients" begin

test/solvers/test_conjugate_gradient.jl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,4 +404,24 @@ using ManifoldDiff: grad_distance
404404
grad_f(M, p) = sum(1 / n * grad_distance.(Ref(M), data, Ref(p)))
405405
@test conjugate_gradient_descent(M, f, grad_f, data[1]) isa PoincareBallPoint
406406
end
407+
408+
@testset "Issue #603: CG with HZ rule on a numerically challenging problem" begin
409+
M = Sphere(2)
410+
p0 = [1.0, 0.0, 0.0]
411+
412+
a = [0.0, 1.0, 0.0]
413+
414+
f(M, p) = 0.5 * norm(p - a)^2
415+
grad_f(M, p) = project(M, p, p - a)
416+
417+
cgs = conjugate_gradient_descent(
418+
M,
419+
f,
420+
grad_f,
421+
p0;
422+
coefficient = ConjugateGradientBealeRestart(HagerZhangCoefficient()),
423+
return_state = true,
424+
)
425+
@test norm(M, cgs.p, grad_f(M, cgs.p)) < 1.0e-8
426+
end
407427
end

0 commit comments

Comments
 (0)