Closed yuwenchen95 closed 9 months ago
Hi, thanks for using Manopt.jl – your error is indeed more related to Manifolds.jl, where both Stiefel and the PowerManifold are defined (Manopt.jl depends only on ManifoldsBase)
May I first ask which version are you using? We made retractions on PowerManifold recently a bit more easy to use, maybe we missed something.
I have two small remarks on the code:
1) If you like, we have a shorthand for power manifold, you can also write M = Stiefel(rnk,d)^100
2) Why are you defining
ManifoldsBase.default_retraction_method(M::Stiefel) = PolarRetraction()
ManifoldsBase.default_retraction_method(M::PowerManifold) = ManifoldsBase.default_retraction_method(M.manifold)
ManifoldsBase.default_inverse_retraction_method(M::Stiefel) = PolarInverseRetraction()
ManifoldsBase.default_inverse_retraction_method(M::PowerManifold) = ManifoldsBase.default_inverse_retraction_method(M.manifold)
usually it is better to use the retraction_method=
keyword of the solver to set the retraction. Your power manifold definitions are actually the default, see for example https://github.com/JuliaManifolds/ManifoldsBase.jl/blob/81d2d93978527120dc95bc628a08f3197a3c6c9c/src/PowerManifold.jl#L458-L463.
But I can recreate your error and take a look (since I am surprised by that myself).
Concerning the packages changed a bit, we had one breaking change in Manifolds (Oct ‘23 version 0.9) in the last 2 years (v0.8 was in May 2022, which the Power Manifold is from. So yes there was a breaking change, but that should not be related to the power manifold. We sure introduced quite a few features over the last year, but we only in very rare cases do braking changes – the version 0l9 introduced more stable and faster storage of parameters in manifolds.
For Manopt.jl the last breaking change was in January, and that was mainly since we migrate function definitions (costs, gradients,...) either to ManifoldDiff.jl or to ManoptExamples.jl, since Manopt.jl should – at some point – only focus on the algorithms and algorithms elements (step sizes, stopping criteria,...); but also here we try to minimise the amount of breaking changes (since we are at 0.4, we had 3 since the beginning in 2016). Of course that is sometimes a tradeoff, but we try to as few as possible breaking changes, the one in January was to introduce the subsolvers properly I think.
edit: if you really want to be sure your 1-year-old code still runs, run it on the same versions as back then. Yes we introduced a small bug in the last change (see below), but that one as also a breaking change – so your old code might also just really break on a breaking change. That is what breaking changes do.
Ha. Found it!
We changed exactly two things in ManifoldsBase 0.15, and you it a combination of these that we missed ;)
and in this rework we also removed an one PowerRetraction
we had for a while. And there we missed one, let‘s say “dispatch-path”. The PR linked directly above fixes this. If you are in a hurry, you can import and define
function ManifoldsBase.retract!(
M::AbstractPowerManifold,
q,
p,
X,
t::Number,
m::AbstractRetractionMethod = default_retraction_method(M, typeof(p)),
)
rep_size = representation_size(M.manifold)
for i in get_iterator(M)
retract!(
M.manifold,
_write(M, rep_size, q, i),
_read(M, rep_size, p, i),
_read(M, rep_size, X, i),
t,
m,
)
end
return q
end
or you wait for ManifoldsBase.jl 0.15.4, which is just missing a test coverage check for now. Or use ManifoldsBase.jl 0.14.x together with Manifolds.jl 0.8.x until then :)
Thanks for pointing this out.
Ha. Found it!
We changed exactly two things in ManifoldsBase 0.15, and you it a combination of these that we missed ;)
- retractions follow since then the “allocate-then-dispatch” mode we do in the rest of the packages for quite some time (it reduced the code by half, and it is now easier to define new retractions)
- Power manifolds moved to ManifoldsBase (so one can use them without loading all of Manifolds.jl).
and in this rework we also removed an one
PowerRetraction
we had for a while. And there we missed one, let‘s say “dispatch-path”. The PR linked directly above fixes this. If you are in a hurry, you can import and definefunction ManifoldsBase.retract!( M::AbstractPowerManifold, q, p, X, t::Number, m::AbstractRetractionMethod = default_retraction_method(M, typeof(p)), ) rep_size = representation_size(M.manifold) for i in get_iterator(M) retract!( M.manifold, _write(M, rep_size, q, i), _read(M, rep_size, p, i), _read(M, rep_size, X, i), t, m, ) end return q end
or you wait for ManifoldsBase.jl 0.15.4, which is just missing a test coverage check for now.
Thanks for pointing this out.
Hi,
I tried the fixing branch and it works now. Thank you so much for the quick update!
Great that that fixed everything already :)
And sure, the retractions are hopefully relatively easy to use – the code behind is a bit tricky, since we try to be flexible and efficient. So that one does not see bugs in that directly, might (but hopefully does not too often) happen. The same for the hopefully relatively easy to use power manifold.
I tried to optimize a quadratic function over a PowerManifold whose submanifolds are a cluster of Stiefel manifolds.
However, it returns the following error:
which seems that I need to define the
retract_polar!()
myself according to this page? I would expect the function can detect the submanifold (Stiefel) and call theretract
function for Stiefel manifolds.Btw, It's a bit confusing to me since I am running the same code I wrote about 1 year ago and there is no error like this one before. The packages related to manifold computation seem to change a lot since then.