Proposal to remove the JuMP Extension#532
Conversation
|
Ah, for the first I found an example call in will continue with and try to understand how that has to be stored then. But it seems to also do a scalar variable? But it is not? |
|
What's the simplest example you want to be able to build? No JuMP. Pure Manopt.jl |
|
The very simplest example is the Riemannian center of mass. Given 3 points compute the minimiser of the squared distances. Hm, but in my first 5 minute hack I also ran into problems there. But as I wrote in slack, last time I gave up on this after 4 months, so this is not meant to be finished fast and I would prefer to maybe at some point at least understand a little bit the code that now lives in my code base. This aims to be an extension where at least I can understand the code. I am aware that this might take much longer than 4 months of me - every now and then - diving somewhere into JuMP code. You hope it fulfils enough other cases as well, I do not understand any of the code - I feel that is not a very sustainable variant of your code living in my package. |
Code? At the moment I'm not even sure if JuMP can support your problem types. We really assume that variables belong to R^n, and that there is such a thing as a scalar variable in R. |
|
Here is some code using Manopt, Manifolds
using ManifoldDiff: grad_distance
M = Hyperbolic(2)
data = PoincareBallPoint.([ [0.1, 0.2], [0.3,0.25], [0.35,0.4] ])
n = length(data)
f(M, p) = sum(1 / (2 * n) * distance.(Ref(M), Ref(p), data) .^ 2)
grad_f(M, p) = sum(1 / n * grad_distance.(Ref(M), data, Ref(p)));
m1 = gradient_descent(M, f, grad_f, data[1]; stepsize=ConstantLength(0.2))Concerning the Rn thing. Sure we can open that discussion as well. About a year or a year and a half ago I was nearly convinced to totally delete this extension, because also then we discussed whether that is useful. Shall we open that again? In the end, none of my problems live in Rn - they can be represented in a set of real numbers usually (like unit norm vectors in R3 are a two-dimensional space represented in R3 but it is not R3)... I am open to deleting the extension, exactly due to the reason that Manopt gos far beyond things in Rn, but @blegat was against that. For the example above, on Hyperbolic space there are three different ways to represent points: one the Hyperboloid (or array default, but also has a type) poincare ball and poincare half plane. All 3 do have a similar interpretation, just because the hyperboloid is the most common, we used that for arrays, the other are distinguished. With variables in R and the total thing in Rn we only exactly have But again. If all this does not fit, then we should probably really delete this extension. I am totally open to that as well. |
|
To not only have the pessimistic side: reasons why it might fit JuMP
But again, if it does not fit, I am totally fine deleting the extension as well. I am not here to try to sneak this package into something where it does not belong. |
|
I'll discuss this with @blegat next week at https://jump.dev/meetings/jumpdev2025/ |
|
Ok. Enjoy JumpDev 2025! |
|
We discussed this at JuMP-dev. A summary:
Benoît might have some other comments, but those were my notes. |
|
Thanks for the detailed discussion points, that I feel are indeed very central to how to continue here.
Currently (and for the foreseeable future) all manifolds are finite dimensional and matrix based.
Do all. these not fit? That would be very unfortunate, since just because it came first in our implementation, the only “nonwrapped” (to be precise not necessarily wrapped) would be the hyperbolic representation. So a main question for me: does matrix-based here refer to mathematically based on a matrix? Then all manifold we have to fit into that.
This is one further point where I would say, then it really does not fit. We nearly always have manual / custom gradients that have to be provided.
I do see, that for you the documentation for a user is of course more central than for a developer, because they are In the vast majority for you.
I tried that the last few times already.
My only point here is, if code lives in “my” repository, I want to be able to provide support to some extend and at least understand it roughly myself. For now I do not do that (see missing documentation above). So maybe the inverse idea of having (if at all) an extension in your responsibility is the better idea. So my conclusion would be to move it to your responsibility, but if the first question is that only arrays should be used, then I would prefer to both delete it here and not do it at all, since the ideas of JuMP and Manopt are just too different in how to approach things |
# Conflicts: # ext/ManoptJuMPExt.jl # test/MOI_wrapper.jl
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev-0.6 #532 +/- ##
===========================================
Coverage ? 100.00%
===========================================
Files ? 90
Lines ? 10897
Branches ? 0
===========================================
Hits ? 10897
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I gave this a careful thought again, and with all the arguments stated and discussion we had, I think it is best to remove the JuMP extension for now. I thought for a while it was cool for visibility, since you really have a large user base, but as a mere mortal it feels impossible to me to maintain such an extension. Furthermore
All these in mind – I am open to have such a “bridge” between Manopt and JuMP somewhere, but better in an own package somewhere in the “JuMP universe”, I am happy to help with manifold-related things. This way around, the parts that are internal and not (yet) so well documented are “closer” to the package. I would carefully claim that both Manifolds.jl and Manopt.jl are reasonably documented – but of course I am also available to help if someone wants to give this connection a new try. I will keep this PR open maybe until somewhen in January 2026 and then merge it. Thanks for all the discussions and efforts. |
Thinking more about this, it may be possible with Riemannian objective (added in #448). But with classical objective for which JuMP is responsible to do the AD, it might be quite complicated.
Moving it to a package either under my personal account or under jump-dev sounds good to me. From my perspective, maintaining it here as a package extension or maintain it in a separate package is mostly the same amount of work so I'm fine with anything. |
|
Then it would be great to have it as a package either on your account or the jump namespace. |
|
I diverted the PR to be collected on a dev-0.6 branch, since we plan to collect a few forthcoming breaking changes for that release. Will merge this when the tests have completed. |
odow
left a comment
There was a problem hiding this comment.
Probably for the best. @blegat and I may revisit if we get around to adding true vector-valued nonlinear operators.
(Un)related: this PR is a great example of why I'm very hesitant to add new extensions in JuMP-related packages. Linking here so I have a back link if I ever need to find this in future: jump-dev/MathOptInterface.jl#2972 and jump-dev/MathOptInterface.jl#2967
|
As I mentioned, I am happy to help from the Manopt-side, if you give it a new try. I personally felt too often lost within JuMP. And that is not meant as negative as it sounds. I think the docs for users of JuMP are great and you do a very good job on the development and commenting (even now here on such short note!), just that from the development side, I often was diving deep into the source code of JuMP itself. To the “vector valued nonlinear operators” – I fear we would especially need variables (and corresponding costs) beyond vectors – into matrices and even arbitrary structs (wrappers of representations or even structured data like for fixed rank an SVD or so). But maybe some day we continue this adventure then :) |
|
Sounds good, I am planning to experiment with vector-valued nonlinear operators in https://github.com/blegat/ArrayDiff.jl/ |
|
I always jump around on manifolds. It indeed sounds a bit bouncy. If so you should name all packages like that (or even use Joke aside (had a long day already since I am currently in JST time zone). Great that you reach out about a name. I am fine with either Back to the start … since if you actually do jump on a trampoline, the surface does form a manifold and it looks even a bit hyperbolic... edit: Trampolin.jl is indeed a name that is available in the General Registry and though that started as a joke when I started writing it, it is somehow nice. relates to JuMP (is a bit bouncy, yes) and curved. |
|
More seriously, regarding |
Edit: during this discussion the title and intention of the PR changed.
I gave the idea of non-array types another try, this is a very first sketch, me diving into an archaeological excatvation in paths through undocumented JuMP internals.
@blegat would you maybe help me in two things?
@variablemyself. An example call would be neat to further play Tomb Raider through JuMP code with functions I would needManopt.jl/ext/ManoptJuMPExt.jl
Lines 613 to 619 in 2fdb644
from
functopoint, but otherwise it is often documented asinfo. What is this, what does it contain? Maybe magically some variable info where I can get the point type again from?I currently have an
@infoin the code to get more of this, but it looks very very very very long and to me pure gibberish:One further thing that is totally unclear to me is whether you ever store tangent vectors (gradients for example). The challenge is then, that this might (in Manifolds) even be of different “shape” than points. Example
SVDMPoint(for fixed rank matrices, the example I would love to be able to provide) – stores an SVD, so U, S, V, where S is a vector (in data structure, mathematically more a diagonal matrix, but stored as a vector)UMVTangentvectors, similar structures for U and V but M is now indeed a n-by-n matrix.Well, hopefully JuMP never cares and this part just works ;)