Skip to content

Commit 920ea2f

Browse files
committed
Adress points from code review.
1 parent 2aedfb2 commit 920ea2f

5 files changed

Lines changed: 10 additions & 14 deletions

File tree

Changelog.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ We also unified a few of the internal solver state constructors.
2020
* New stopping criteria: `StopWhenRelativeAPosterioriCostChangeLessOrEqual` and `StopWhenProjectedNegativeGradientNormLess`. (#554).
2121
* `HagerZhangLinesearch` stepsize, a state-of-the-art line search for smooth objectives with cubic interpolation and adaptive Wolfe condition checking. (#554)
2222
* Stopping criteria can now be initialized using `initialize_stepsize!`, similar to solvers. (#554)
23+
* The `ConjugateResidualState` now has a `warm_start=` option when used multiple times, for example in every iteration as a subsolver, to reuse the last state from the previous run.
2324

2425
### Changed
2526

2627
* In the [Riemannian Levenberg Marquardt algorithm](https://manoptjl.org/stable/solvers/LevenbergMarquardt/)t the `η` parameter has been renamed to `candidate_acceptance_threshold`, `β` to `damping_increase_factor` and `β_reduction` to `damping_reduction_factor`. (#617)
2728
* the constructor for the [Levenberg-Marquardt state](https://manoptjl.org/stable/solvers/LevenbergMarquardt/#Manopt.LevenbergMarquardtState) has been unified with the remaining states, to take the `sub_problem` and `sub_state` arguments as second and third positional arguments, respectively. (#617)
2829
* the keyword `initial_jacobian_f` within `LevenbergMarquardt` is unified in naming to the residual values vector and called `initial_jacobian_matrices`. If you call `LevenbergMarquardt` with a single vector component, also a single matrix is allowed. (#617)
2930
* an internal field of the solver state of Levenberg-Marqwuardt was called `jacobian_f` the same as the functions whose result it meant to cache if applicable. To distinguish both, the field is now called `jacobian_matrices`. (#617)
30-
* title of "How to define the cost in the embedding" tutorial (#615)
31+
* the `max_stepsize(M)` on the [`SymmetricPositiveDefinite`](https://juliamanifolds.github.io/Manifolds.jl/stable/manifolds/symmetricpositivedefinite/) manifold was changed from returning `Inf`, which is the mathematical maximal stepsize to returning the square root of the maximum (floating point) value to avoid numerical instabilities.
3132

3233
### Fixed
3334

docs/src/notation.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ with the following additional parts.
99
| ``\mathbf{A}`` | a matrix | ``\mathbf{B}, \mathbf{C}`` | |
1010
| ``\mathcal{A}, \mathcal{L}`` | linear operators, usually on tangent spaces | | |
1111
| ``\mathcal{C}`` | a (geodesically) convex set | | |
12-
| ``\operatorname*{arg\,min}_v`` | argument/variable ``v`` of a function ``f`` where a local or global minimum is attained | |
12+
| ``\operatorname*{arg\,min}_v f(v)`` | argument/variable ``v`` of a function ``f`` where a local or global minimum is attained | |
1313
| ``k`` | the current iterate | ``i`` | the goal is to unify this to `k` |
1414
| ```` | The [Levi-Cevita connection](https://en.wikipedia.org/wiki/Levi-Civita_connection) | | |

docs/src/plans/objectives/constrained.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ ConstrainedManifoldObjective
99
ManifoldConstrainedSetObjective
1010
```
1111

12-
It might be beneficial to use the adapted problem to specify different ranges for the gradients of the constraints
13-
1412
```@docs
1513
ConstrainedManoptProblem
1614
```

src/plans/robustifiers.jl

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Evaluate the robustifier ``ρ`` and its first two derivatives at `x`.
3030
A tuple `(a, b, c)` with
3131
* `a = ρ(x)`
3232
* `b = ρ'(x)`
33-
* `c = ρ''(x)` (might be `missing` if not defined)
33+
* `c = ρ''(x)`
3434
"""
3535
get_robustifier_values::AbstractRobustifierFunction, x::Real)
3636

@@ -52,17 +52,14 @@ and second derivative ``ρ'`` and ``ρ''``, respectively.
5252
5353
Generate a `RobustifierFunction` given the function `ρ` and its first and second derivative.
5454
"""
55-
struct RobustifierFunction{F <: Function, G <: Function, H <: Union{Function, Missing}} <: AbstractRobustifierFunction
55+
struct RobustifierFunction{F <: Function, G <: Function, H <: Function} <: AbstractRobustifierFunction
5656
ρ::F
5757
ρ_prime::G
5858
ρ_double_prime::H
5959
end
6060

6161
function get_robustifier_values(ρf::RobustifierFunction, x::Real)
62-
a = ρf.ρ(x)
63-
b = ρf.ρ_prime(x)
64-
c = ismissing(ρf.ρ_double_prime) ? missing : ρf.ρ_double_prime(x)
65-
return (a, b, c)
62+
return (ρf.ρ(x), ρf.ρ_prime(x), ρf.ρ_double_prime(x))
6663
end
6764

6865
#
@@ -204,7 +201,7 @@ function get_robustifier_values(
204201
(a1, b1, c1) = get_robustifier_values(crf.ρ1, a2)
205202
a = a1
206203
b = b1 * b2
207-
c = (ismissing(c1) || ismissing(c2)) ? missing : c1 * b2^2 + b1 * c2
204+
c = c1 * b2^2 + b1 * c2
208205
return (a, b, c)
209206
end
210207

@@ -314,7 +311,7 @@ function get_robustifier_values(srf::ScaledRobustifierFunction, x::Real)
314311
(a, b, c) = get_robustifier_values(srf.robustifier, x / s2)
315312
a_scaled = s2 * a
316313
b_scaled = b
317-
c_scaled = ismissing(c) ? missing : c / s2
314+
c_scaled = c / s2
318315
return (a_scaled, b_scaled, c_scaled)
319316
end
320317

src/solvers/LevenbergMarquardt.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ as well as in general using the model improvement parameter ``m_k`` in several p
7777
* `initial_jacobian_matrices`: a cache for the evaluated Jacobians (currently only used if `use_unified_basis = true`, then initialised to a vector of jacobian matrices, otherwise ignored)
7878
$(_kwargs(:retraction_method))
7979
* `scaling_threshold = 1.0e-6`: a threshold `ε` to bound the scaling parameter `α` in the robust case away from `1`, see [`get_LevenbergMarquardt_scaling`](@ref)
80-
* `scaling_mode = :Default`: specify the scaling stabilization mode, see [`get_LevenbergMarquardt_scaling`](@ref)
80+
* `scaling_mode = :Strict`: specify the scaling stabilization mode, see [`get_LevenbergMarquardt_scaling`](@ref)
8181
$(_kwargs(:stopping_criterion; default = "`[`StopAfterIteration`](@ref)`(500)`$(_sc(:Any))[`StopWhenGradientNormLess`](@ref)`(1.0e-12)`$(_sc(:Any))[`StopWhenStepsizeLess`](@ref)`(1.0e-12)"))
8282
* `sub_objective` : specify the objective for the surrogate sub problem to solver in every iteration.
8383
This is set depending on the `use_unified_basis`
@@ -238,7 +238,7 @@ function LevenbergMarquardt!(
238238
fill(nothing, length(get_objective(nlso).objective))
239239
end,
240240
scaling_threshold::Real = 1.0e-6,
241-
scaling_mode::Symbol = :Default,
241+
scaling_mode::Symbol = :Strict,
242242
minimum_acceptable_model_improvement::Real = eps(number_eltype(p)),
243243
sub_objective = construct_lm_subobjective(use_unified_basis, nlso, damping_term_min, scaling_threshold, scaling_mode, initial_residual_values, initial_jacobian_matrices),
244244
sub_problem = DefaultManoptProblem(TangentSpace(M, p), sub_objective),

0 commit comments

Comments
 (0)