Differentiate more precisely bewteen swarm and iterate in PSO more st…

…ricly (#344)

* Differentiate more precisely bewteen swarm and iterate.
* increase test coverage.
* Rename the generic name parameter :Swarm to :Population
* clarify symbols, rename swarm to population.

Changelog.md

@@ -5,6 +5,14 @@ All notable Changes to the Julia package `Manopt.jl` will be documented in this
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.4.48]
+
+### Fixed
+
+* fixes an imprecision in the interface of `get_iterate` that sometimes led to the swarm of `particle_swarm` being returned as the iterate.
+* refactor `particle_swarm` in naming and access functions to avoid this also in the future.
+  To access the whole swarm, one now should use `get_manopt_parameter(pss, :Population)`
+
 ## [0.4.47] January 6, 2024
 
 ### Fixed

Project.toml

@@ -1,7 +1,7 @@
 name = "Manopt"
 uuid = "0fc0a36d-df90-57f3-8f93-d78a9fc72bb5"
 authors = ["Ronny Bergmann <[email protected]>"]
-version = "0.4.47"
+version = "0.4.48"
 
 [deps]
 ColorSchemes = "35d6a980-a343-548e-a6ea-1d62b119f2f4"

docs/src/plans/index.md

@@ -29,18 +29,22 @@ The column “generic” refers to a short hand that might be used for readabil
 | Symbol       | Used in | Description                                                | generic |
 | :----------- | :------: | ;-------------------------------------------------------- | :------ |
 | `:active` | [`DebugWhenActive`](@ref) | activity of the debug action stored within | |
-| `:Basepoint` | [`TangentSpace`]() | the point the tangent space is at           | `:p` |
+| `:Basepoint` | [`TangentSpace`]() | the point the tangent space is at           | often `:p` |
 | `:Cost` | generic |the cost function (within an objective, as pass down) | |
 | `:Debug` | [`DebugSolverState`](@ref) | the stored `debugDictionary` | |
-| `:Gradient` | generic |the gradient function (within an objective, as pass down) | |
+| `:Gradient` | generic | the gradient function (within an objective, as pass down) | |
 | `:Iterate` | generic | the (current) iterate, similar to [`set_iterate!`](@ref), within a state | |
 | `:Manifold` | generic |the manifold (within a problem, as pass down) | |
 | `:Objective` | generic | the objective (within a problem, as pass down) | |
 | `:SubProblem` | generic | the sub problem (within a state, as pass down) | |
 | `:SubState` | generic | the sub state (within a state, as pass down) | |
 | `:λ` | [`ProximalDCCost`](@ref), [`ProximalDCGrad`](@ref) | set the proximal parameter within the proximal sub objective elements | |
-| `:p`         | generic | a certain point         | |
-| `:X`         | generic | a certain tangent vector | |
+| `:Population`     | [`ParticleSwarmState`](@ref) | a certain population of points, e.g. [`particle_swarm`](@ref)s swarm | |
 | `:TrustRegionRadius` | [`TrustRegionsState`](@ref) | the trust region radius | `:σ` |
 | `:ρ`, `:u` | [`ExactPenaltyCost`](@ref), [`ExactPenaltyGrad`](@ref) | Parameters within the exact penalty objective | |
 | `:ρ`, `:μ`, `:λ` | [`AugmentedLagrangianCost`](@ref) and [`AugmentedLagrangianGrad`](@ref) | Parameters of the Lagrangian function | |
+
+Since the iterate is often stored in the states fields `s.p` one _could_ access the iterate
+often also with `:p` and similarly the gradient with `:X`.
+This is discouraged for both readability as well as to star more generic, and it is recommended
+to use `:Iterate` and `:Gradient` instead in generic settings.

src/Manopt.jl

@@ -320,7 +320,6 @@ export get_state,
     forward_operator,
     forward_operator!,
     get_objective
-export set_manopt_parameter!
 export get_hessian, get_hessian!
 export ApproxHessianFiniteDifference
 export is_state_decorator, dispatch_state_decorator

src/plans/plan.jl

@@ -14,8 +14,6 @@ status_summary(e) = "$(e)"
 
 For any `f` and a `Symbol` `e` we dispatch on its value so by default, to
 set some `args...` in `f` or one of uts sub elements.
-
-
 """
 function set_manopt_parameter!(f, e::Symbol, args...)
     return set_manopt_parameter!(f, Val(e), args...)
@@ -24,6 +22,19 @@ function set_manopt_parameter!(f, args...)
     return f
 end
 
+"""
+    get_manopt_parameter(f, element::Symbol, args...)
+
+For any `f` and a `Symbol` `e` we dispatch on its value so by default, to
+get some element from `f` potentially further qulalified by `args...`.
+
+This functions returns `nothing` if `f` does not have the property `element`
+"""
+function get_manopt_parameter(f, e::Symbol, args...)
+    return get_manopt_parameter(f, Val(e), args...)
+end
+get_manopt_parameter(f, args...) = nothing
+
 include("objective.jl")
 include("problem.jl")
 include("solver_state.jl")

src/plans/solver_state.jl

@@ -190,6 +190,8 @@ end
     get_iterate(O::AbstractManoptSolverState)
 
 return the (last stored) iterate within [`AbstractManoptSolverState`](@ref)` `s`.
+This should usually refer to a single point on the manifold the solver is working on
+
 By default also undecorates the state beforehand.
 """
 get_iterate(s::AbstractManoptSolverState) = _get_iterate(s, dispatch_state_decorator(s))
@@ -202,16 +204,6 @@ function _get_iterate(s::AbstractManoptSolverState, ::Val{false})
 end
 _get_iterate(s::AbstractManoptSolverState, ::Val{true}) = get_iterate(s.state)
 
-"""
-    get_manopt_parameter(ams::AbstractManoptSolverState, element::Symbol, args...)
-
-Obtain a certain field or semantic element from the [`AbstractManoptSolverState`](@ref) `ams`.
-This function passes to `Val(element)` and specific setters should dispatch on `Val{element}`.
-"""
-function get_manopt_parameter(ams::AbstractManoptSolverState, e::Symbol, args...)
-    return get_manopt_parameter(ams, Val(e), args...)
-end
-
 _set_iterate!(s::AbstractManoptSolverState, M, p, ::Val{true}) = set_iterate!(s.state, M, p)
 
 """
@@ -570,6 +562,11 @@ function update_storage!(
             a.values[key] = deepcopy(get_iterate(s))
         elseif key === :Gradient
             a.values[key] = deepcopy(get_gradient(s))
+        elseif key === :Population
+            swarm = get_manopt_parameter(s, key)
+            if !isnothing(swarm)
+                a.values[key] = deepcopy.(swarm)
+            end
         else
             a.values[key] = deepcopy(getproperty(s, key))
         end

src/plans/stopping_criterion.jl

@@ -206,7 +206,7 @@ end
 function StopWhenChangeLess(
     M::AbstractManifold,
     ε::Float64;
-    storage::StoreStateAction=StoreStateAction(M; store_points=Tuple{:Iterate}),
+    storage::StoreStateAction=StoreStateAction(M; store_points=Tuple{:Iterate,:Population}),
     inverse_retraction_method::IRT=default_inverse_retraction_method(M),
 ) where {IRT<:AbstractInverseRetractionMethod}
     return StopWhenChangeLess{IRT,typeof(storage)}(
@@ -215,12 +215,12 @@ function StopWhenChangeLess(
 end
 function StopWhenChangeLess(
     ε::Float64;
-    storage::StoreStateAction=StoreStateAction([:Iterate]),
+    storage::StoreStateAction=StoreStateAction([:Iterate, :Population]),
     manifold::AbstractManifold=DefaultManifold(),
     inverse_retraction_method::IRT=default_inverse_retraction_method(manifold),
 ) where {IRT<:AbstractInverseRetractionMethod}
     if !(manifold isa DefaultManifold)
-        @warn "The `manifold` keyword is deprecated, use the first positional argument `M`. This keyword for now sets `inverse_retracion_method`."
+        @warn "The `manifold` keyword is deprecated, use the first positional argument `M` instead."
     end
     return StopWhenChangeLess{IRT,typeof(storage)}(
         ε, "", storage, inverse_retraction_method, 0