JuliaManifolds / Manifolds.jl

Manifolds.jl provides a library of manifolds aiming for an easy-to-use and fast implementation.
https://juliamanifolds.github.io/Manifolds.jl
MIT License
368 stars 53 forks source link

Integration into Optim #35

Open antoine-levitt opened 5 years ago

antoine-levitt commented 5 years ago

Hi,

I just checked back, it seems there has been lots of activity on this project, great! Is it ready for prime time? In particular, can we replace Optim's implementation of manifolds with this package? We can think about a deeper integration sometime in the future, but right now I think just supporting manifolds embedded in R^n using retraction and projection of a vector onto the tangent space for the Stiefel and sphere manifolds (the basic things implemented in https://github.com/JuliaNLSolvers/Optim.jl/blob/master/src/Manifolds.jl) with a stable-ish API will be enough.

@pkofod: or should we wait until the next-gen optim?

mateuszbaran commented 5 years ago

Thanks for asking!

I think Manifolds.jl isn't quite ready yet. A few things we need to consider before using it in things like Optim.jl:

  1. Stiefel manifold isn't done yet (but it shouldn't be problematic), also the power manifold is a work in progress. I want to do it right (that's why I'm working on hybrid dynamic-static arrays in this branch: https://github.com/JuliaNLSolvers/Manifolds.jl/tree/mbaran/power-manifold , I'll put them in a separate package soon).
  2. Documentation isn't deployed yet and we need a better README.
  3. Manifolds.jl, at least in its current shape, is a pretty heavy dependency for something as simple as projection and retraction. A large part of this is caused by manifold-valued distributions (which could be moved into a separate package) and the metric manifold.
  4. Our API for basic operations (including retraction) looks pretty stable. Projection onto the tangent space might receive a new argument argument that indicates the method to be used (like for retractions) but API should be backward-compatible with the current one.

I think issues 1-3 should be solved before Manifolds.jl can be safely used in Optim.jl (or the new Optim). I need a few weeks to solve (1). Issue (3) needs some discussion, I'm not sure how important a low number of dependencies is for you.

pkofod commented 5 years ago

@pkofod: or should we wait until the next-gen optim?

I think it's fine to try it out in a system we know :)

pkofod commented 5 years ago

I'm not sure how important a low number of dependencies is for you.

Not important if it's "our own"

kellertuer commented 4 years ago

Thanks for this topic, I was not able to continue working the last month (sadly), I will continue to work on the SPDs hopefully next week – but I am on @mateuszbaran s side considering the basic API, but I would love to have at least a proper, complete documentation as well as maybe a few more manifolds before integrating somewhere.

pkofod commented 4 years ago

I think issues 1-3 should be solved before Manifolds.jl can be safely used in Optim.jl (or the new Optim). I need a few weeks to solve (1). Issue (3) needs some discussion, I'm not sure how important a low number of dependencies is for you.

I've been reading the code in this package and thinking about using it in Optim going forwards. I'm super impressed, and the work seems super thorough. I still don't see an immediate need to split this package into separate packages; if you're opting in to one manifold, let's just provide all the batteries you could ever have thought of! But, and let me just undermine my own statement one sentence later, I think we may want to create just one additional package that is the "ManifoldsBase.jl".

What is ManifoldsBase.jl? ManifoldsBase.jl has all the stubs, the generic interface, and so on. And quite importantly, it also has the Euclidian manifold. I am saying this because I obviously want numerical analysis packages to take advantage of all this good work, but I don't think many packages will want to include Manifolds in their Project.jl file, because it is indeed a bit heavy. How long does it take to load for you guys? It's 16 seconds on my windows machine, haven't tested on linux. Is that OrdinaryDiffEq? Anyway, Optim et al should just be allowed to include the Euclidian manifold and the full API, and then people can implement their own manifolds or using Manifolds to get the full suite. Optim cannot take on a dependency that takes 16 seconds to load, especially because Optim itself is a dependency of many packages in the ecosystem, and few of those packages use manifods, so it would be a shame to push 10 seconds of addition using times onto them :)

antoine-levitt commented 4 years ago

Possibly another option would be to load ODE lazily if required?

Also speaking of that, something that came up with @jagot is that he wanted to do a crazy manifold where he keeps some columns of a matrix orthogonal and some others normalized, the splitting being non-contiguous. What that illustrates is that it's important that users be able to define a manifold with just the two operations Optim needs, and not necessarily all the operations supported by this package.

mateuszbaran commented 4 years ago

Thanks, we are trying to make this package as good as we can :slightly_smiling_face: .

On my computer it takes 24 seconds to load Manifolds, that's way too much for a dependency. OrdinaryDiffEq takes 18 seconds. What ManifoldsBase.jl would have to include without redesign of our interface:

This should take the loading time down to perhaps 3 seconds, maybe a bit less. That's about as much as using Optim takes for me. Do you think it'd be OK to require SimpleTraits unconditionally?

What that illustrates is that it's important that users be able to define a manifold with just the two operations Optim needs, and not necessarily all the operations supported by this package.

Yes, no one has to implement the full interface if they don't need it. Even many of our manifolds don't implement everything we have in our generic interface tests. Extending our functions would be totally fine IMHO.

pkofod commented 4 years ago

Why is Distributions needed?

edit: let me clarify, I'm not in doubt about what it's used for, I'm just curious why it would be required in defining the stubs.

mateuszbaran commented 4 years ago

We extend their VariateForm and ValueSupport as well as derive from Distribution: https://github.com/JuliaNLSolvers/Manifolds.jl/blob/master/src/DistributionsBase.jl .

pkofod commented 4 years ago

Okay, so it is technically possible to just only support sampling if using Manifolds, right?

mateuszbaran commented 4 years ago

Right, that sounds reasonable. I'll add separating ManifoldsBase.jl to my TODO list :slightly_smiling_face: .

sethaxen commented 4 years ago

I also think ManifoldsBase.jl is a good idea and should definitely include Euclidean.

I think issues 1-3 should be solved before Manifolds.jl can be safely used in Optim.jl (or the new Optim). I need a few weeks to solve (1).

Unfortunately, I've not had the time to continue work on JuliaManifolds/Manifolds.jl#25 and probably won't in the immediate future. Defining defaults for Lie groups is tricky.

Possibly another option would be to load ODE lazily if required?

This is probably wise even for Manifolds.jl.

mateuszbaran commented 4 years ago

I've looked at this again and essentially the last missing piece is gradient calculation. Optim.jl currently just projects Euclidean gradient to the tangent space but it doesn't work for some manifolds (see for example JuliaManifolds/ManifoldDiff.jl#27).

antoine-levitt commented 4 years ago

Optim uses manifolds as an opportunity feature - it takes a few lines of code to implement so why not. I think it's fine to keep Optim as a "poor man's manifold optimization" since the focus is on unconstrained optimization. Manopt.jl is there for more serious manifoldization. Unless there is a will to merge both packages?

kellertuer commented 4 years ago

I think that's a good approach, since Manopt.jl– as I think we already discussed – will focus on Optimization on Manifolds including constrained and nonsmooth (theses especially) problems. Further I would like to stay in the area of manopt and pymanopt, where we currently join forces across languages.

kellertuer commented 1 year ago

This has been inactive quite a while; on the other hand our API (ManifoldsBase.jl) is really stable by now – so besides the differences in the notion of a retraction, ManifoldsBase.jl could be used in Optim maybe (still staying on the opportunity/“poor mans variant” within Optim)?

antoine-levitt commented 1 year ago

Yeah I think the status has been "oh yeah, somebody should really do it at some point" for a few years now :-)

kellertuer commented 1 year ago

That is great to know – and don't worry, I know that status just too well. Then let's leave this issue open until this really actually happens – maybe I find the time as well, but I am not so experienced with the inner organisation of the Optim.jl code

mateuszbaran commented 1 year ago

A small update: I've made a script that allows for using Manifolds.jl manifolds in current Optim.jl: https://gist.github.com/mateuszbaran/0354c0edfb9cdf25e084a2b915816a09 . Changing Optim.jl to using Manifolds.jl manifolds without a wrapper would be a breaking change with minimal benefits over using that wrapper. IMHO the best short and medium-term path forward is putting the glue code in some package (either Optim.jl or Manifolds.jl) as a package extension (a new Julia 1.9 feature). What do you think?

kellertuer commented 1 year ago

Just a question for understanding – with this wrapper Optim would do “addition in the embedding and projection” in its solvers?

I like the glue code idea especially since it is quite short. Maybe we can do an Issue in Optim to do the breaking change for the next larger update (that is when they have a breaking change anyways)?

I would maybe put the glue stuff it into Optim, though I do not have a very good argumen, just a feeling – since the code extends the manifold-capabilities of Optim.

mateuszbaran commented 1 year ago

Just a question for understanding – with this wrapper Optim would do “addition in the embedding and projection” in its solvers?

Yes, that's what Optim.jl does. And also projecting tangent vectors.

I would maybe put the glue stuff it into Optim, though I do not have a very good argumen, just a feeling – since the code extends the manifold-capabilities of Optim.

Putting it in Optim.jl would make sense IMO but let's wait for an opinion of Optim maintainers.

kellertuer commented 1 year ago

Does it maybe make sense to post this idea also at https://github.com/JuliaNLSolvers/Optim.jl/issues/448 ?

mateuszbaran commented 1 year ago

I think that topic is more about Optim.jl doing more than just retract and project_tangent and that's a different issue.

kellertuer commented 1 year ago

ah, I see, right.

pkofod commented 1 year ago

I think it would make sense. I do not have too much bandwidth, but I'd love to help if I can. Optim.jl is fine for me. If we need to break, let's discuss.