Skip to content

Proof of concept for non-array point - #491

Merged
blegat merged 1 commit into
JuliaManifolds:kellertuer/fix-313-a-jump-tutorialfrom
blegat:bl/hyperboloid
Jul 31, 2025
Merged

blegat merged 1 commit into
JuliaManifolds:kellertuer/fix-313-a-jump-tutorialfrom
blegat:bl/hyperboloid

Conversation

@blegat

@blegat blegat commented Jul 19, 2025

Copy link
Copy Markdown
Collaborator

This gives

julia> @variable(model, x in Hyperbolic(2), start = HyperboloidPoint([1, 2]))
HyperboloidPoint(VariableRef[_[5], _[6]])

What are the operations we could do on x that would result in a scalar objective function ?

@blegat
blegat changed the base branch from master to kellertuer/fix-313-a-jump-tutorial July 19, 2025 18:18
@codecov

codecov Bot commented Jul 19, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 27.27273% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.65%. Comparing base (7ccb6f8) to head (d483e99).
⚠️ Report is 2 commits behind head on kellertuer/fix-313-a-jump-tutorial.

Files with missing lines Patch % Lines
ext/ManoptJuMPExt.jl 33.33% 6 Missing ⚠️
ext/ManoptJuMPManifoldsExt.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                           @@
##           kellertuer/fix-313-a-jump-tutorial     #491      +/-   ##
======================================================================
- Coverage                               99.67%   99.65%   -0.02%     
======================================================================
  Files                                      86       86              
  Lines                                    9460     9384      -76     
======================================================================
- Hits                                     9429     9352      -77     
- Misses                                     31       32       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kellertuer

kellertuer commented Jul 19, 2025

Copy link
Copy Markdown
Member

I do not understand the question, sorry. Maybe call the cost function?

edit: Or doe you mean which functions are implemented specifically for this? All of them. Exp, log, (replace +-) retractions (approximate exp/log)....
But the prior PR therefore “unwrapped” the variables before going to the state/solver.

Comment thread ext/ManoptJuMPExt.jl
# TODO: Understand parameters here and document them
function JuMP.add_variable(model::JuMP.AbstractModel, v::ManifoldVariable, name::String="")
return nothing
function JuMP.add_variable(model::JuMP.AbstractModel, v::ManifoldVariable, ::String="")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe have a it of documentation here? The official docs mainly state “please ask a developer”. If that code is here in Manopt I am a developer and by now I tried for about half a year maybe to understand something – and I simply do not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...your docs do not help https://jump.dev/JuMP.jl/stable/api/JuMP/#JuMP.add_variable – they state I should ask you, and neither are my code archeology skill that good nor have all my tries of that not lead to me understanding how this is supposed to work, nor what for example the string is.

@kellertuer

Copy link
Copy Markdown
Member

This concept also can not work, since the point type is not “available” in Manopt. We can not just “rewrite” the existing parts.

The existing one is an extension

  • Manopt & JuMP

But for specific manifold we need an extension to

  • Manopt, Manifold & JuMP

The Other PR defines the Manifolds-type-wrapper in the first extension and tries a proof of concept in the second.
I am not yet sure how this idea could be fit into this general setting, nor have I understood how it is meant to work.

@blegat

blegat commented Jul 20, 2025

Copy link
Copy Markdown
Collaborator Author

Yes I mean what would be a typical objective I can use in the tests.

But the prior PR therefore “unwrapped” the variables before going to the state/solver.

The solver still receives them unwrapped, we'll have to pass the point type as well. The only difference here is that they are wrapped at the JuMP level so the user will be able to use the operators you have define for HyperboloicPoint to build the objective function

@kellertuer

Copy link
Copy Markdown
Member

Hm, I still do not follow. We do not do much magic with the cost (I think you do).

So I am not sure what “the operators” refers to some “build” of an objective function. We do not “build” anything here.

The user really only implements a cost(M,p) where M is a manifold and p is a point thereon. Maybe Manopt is much much simpler and much less what you think in this sense?

I am still not sure what need to be done here then. Nor do I understand the approach to an extend that I can adapt it for the PR.

But I understand that this is never meant to be merged but just a proof of concept (that I would have added as a comment in the other PR).

@blegat

blegat commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator Author

I just meant : if you give le an example of cost function, just the way you write it with Manopt, I can turn this proof of concept into a working state :)

@kellertuer

kellertuer commented Jul 22, 2025

Copy link
Copy Markdown
Member
function f(M::Hyperbolic, p::HyperboloidPoint) # though this is internally the same as arrays actually
    #compute something based on p.value, I am not creative enough right now... ;)
    return 7.0
end

gradients the same grad_f(M, p) ...but we also support in-place ones: grad_f!(M, X, p) that compute in-place of X. not sure jump can support that.

@kellertuer

Copy link
Copy Markdown
Member

Since we now can both push to #454, we could close this and add it directly there?

@blegat
blegat merged commit 5b84c45 into JuliaManifolds:kellertuer/fix-313-a-jump-tutorial Jul 31, 2025
@blegat

blegat commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator Author

Yes, I just merged it into it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants