-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modularise Constraints #386
Conversation
This is conceptually a quite interesting PR. I just spent a bit of time sketching the new idea of e vectorial objective. I think it would reduce code duplication (between inequality constraint access, equality constraint access and all Levenberg Marquardt) quite a bit. Since I am not 100% familiar with LM; a bit of a feedback would be nice on the ideas sketched here @mateuszbaran: https://manoptjl.org/previews/PR386/plans/objective/#Manopt.VectorialGradientObjective If this sounds good, I could start implementing all that. Note that the basis in LM is now even stored in the type, so it is only there if the type requires it (the CoefficientType) |
This is a nice direction of improvement. I don't quite get the difference between Also, you should be careful to note that
That sounds like a good idea. |
Component is the old one, so basically nested vector, I kept that because I think the power manifold one might (more often) require and actual power manifold while the old vector variant did not. And sure, one idea would be to have this also inside a VectorOptimisation problem later, but maybe we should then rethink the name, if you feel that might be confusing. I would maybe thing that a |
I'm not sure if keeping that separation is actually useful.
Multi-objective optimization doesn't have the function |
I never want to specify said g in this PR since the goal is really to only represent elements that map into Rn, like the equality, or inequality constraints or the vectorial function in LM. At least the first two never have said g. But wrapping this in a new Objective that provides g would be the way to go I think. Ok, doing just the Powermanifold thing should be fine as well and we can omit the component one. |
That's fine but then maybe let's use a name without |
Interesting idea. For a bit of background: This started, when a student of mine said, it might be nice to have a Hessian with the cost in the constrained objective. So I encapsuled that and instead of saving So I would be fine with the idea that the constraints g and h are objectives (though vectorial) themselves. I also do not have enough experience in vectorial optimization whether they sometimes would really just have a vectorial cost? maybe the g you used to get a number is something that is just parametrised in an objective and not a concrete function? Then VectorialObjective would indeed be fine and describing well what we have – a function (and derivative information) that maps into a vector space. Wel, but I am also fine giving it another name, just that I struggle a bit with a good name for now. Do you have ideas for a name? |
Maybe just
Yes, that's what multi-objective optimization deals with. The goal is to explore the Pareto front. It is, in a way, equivalent to exploration of the impact of
I don't understand, why wouldn't it be a concrete function? |
Maybe some vector optimisation area I do not know? I do not know much. But then the vectorial objective we have here is fine for vector optim as well just that a vecetorobjetive need a vectorial objective plus g So I till neither see what would be wrong with the vectorial objective nor do I have any other good name here. |
Yes, it is fine for multi-objective/vector-valued optimization but to me using it directly for anything else is confusing.
Using both names (vector objective and vectorial objective) for different things sounds confusing. Maybe one of this things could be names |
Though not yet used, once we go for vector optimisation I want to keep Since we already discussed this is in most vases (even for LM) just a part of the objective, we could call the type here edit: SplitObjective sounds too vague for me. |
OK, then what about |
Sounds good. Will work on that tomorrow then. Thanks for the feedback and the discussions :) |
…mplementing accessors.
I did the renaming and will now start to write the access functions (which will simplify the 3 existing access functions it will replace quite a bit), |
A final thing to maybe consider: For the power manifold approach one sometimes needs the power manifold to access the elements of the (power manifolds) tangent vector. For now I just added the power representation type to the new PowerManifoldVectorialType. That would, however, mean, one would often generate the power manifold just to access a component. |
It could be an objective for a multi-objective optimization problem but I don't think we want to design that feature in this PR so maybe let's not make it
I will think about it. |
I agree on the first. For the second I have 2 ideas
I am more tending to the first case, since I think the memory/time spend on creating that is not too bad – and the second would break quite a bit with the current model ideas. |
Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
I illustrated my problem a bit with the |
Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
# Conflicts: # Changelog.md
…:JuliaManifolds/Manopt.jl into kellertuer/structural_rework_constraints
Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
…:JuliaManifolds/Manopt.jl into kellertuer/structural_rework_constraints
I did a whole bunch of minor improvements: fixing typos, adding type bounds, a couple of missing tests. Let me know if there is anything you don't like. By the way, it looks like using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any more design comments here -- it looks solid. Just a bit of polishing. It should be possible to do further optimization work incrementally in the future.
Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
Thanks, having a solid design was my main goal. Performance is probably something we can look at in the future then. |
Yes I think I started with having I for inequality and j vor eq and got that mixed up a bit later. Hope that is not too bad – if that is confusing we should make that more consistent, but for me that Is also fine, as is now, since that never mixes anywhere. |
Wow, thanks for fixing all those :) they all look like very great fixes; for bounds I am sometimes not sure when they are useful and when we can skip them, so I trust your expertise here. |
That's not a big issue but there were a few places where docstring and the method it was attached to used different conventions, or type signature in docstring and docstring text. Anyway, that's a minor consistency issue.
You're welcome :). I add bounds for type parameters mainly because then I know what to expect there. If a field is a function or a functor object, I prefer no bound. Otherwise I prefer to have a bound unless there is a good reason against it (for example one reason may be wrapping objects from weak dependencies). |
This is a start to rework constraints, make them a bit more flexible and address / resolve #185.
All 3 parts, the function$f$ , inequality constraints $g$ and equality constraints $h$ are now stored in their own objectives internally. That way, they can be more flexibly provided – for example a Hessian for one of them.
Besides that, there is more details available on the co-domain of these, especially in case of a single function, the gradient can now also be specified to map into the tangent space of the power manifolds tangent space.
One open point still to check is, how the internal functions can be adopted to this in hopefully a non-breaking way.
A main challenge (or where I stopped today), is, when a function that returns gradients (or a gradient of functions) is stored in an objective – and it hence might be cached – how to best model element access.
This might also be a nice improvement (or a unification with) the LevenbergMarquart type objective.
One could unify that to an objective of type
VectorObjective
maybe.Comments on this idea welcome.
🛣️ Roadmap
👨💻 unification with the LestSquaresObjetice (VectorObjective
?)range
, e.g. ALM and EPM grad.DefaultManifoldProblem
passes in a rangenothing
to trigger the defaultget_gradients
toget_gradient
withColon
ConstrainedManifoldObjective
thatg
andgrad_g
are internally stored as the newVectorialGradientFunction
h
andgrad_h
are internally stored as a mewVectorialGradientFunction