JuliaManifolds / Manopt.jl

🏔️Manopt. jl – Optimization on Manifolds in Julia
http://manoptjl.org
Other
314 stars 40 forks source link

Built-in stopping criterions allocate new error strings on each check, even when not required #389

Closed bvdmitri closed 2 months ago

bvdmitri commented 3 months ago

During the optimization of some of my code, I noticed that the built-in stopping criteria in Manopt are less efficient than they could be. Specifically, they allocate memory every time they are called.

image

Notice the very large portion of allocations is being spend inside the string and print_to_string functions. And, no, I don't do any optimizations in the string manifold 😅 . The rest of the allocations are most probably related to my code.

Interestingly, the allocation profile shows that most of my allocations are from the error messages generated by the stopping criteria, as seen here. The issue is that these error messages are allocated every time, even when they are not needed. This occurs because of the way Julia's string interpolation works.

I believe replacing:

c.reason = "The algorithm reached its maximal number of iterations ($(c.maxIter)).\n"

with

c.reason = lazy"The algorithm reached its maximal number of iterations ($(c.maxIter)).\n"

in all the places will address this inefficiency.

P.S. Great package, by the way!

mateuszbaran commented 3 months ago

I don't like this eager rendering of reason either. We have the get_reason function which should handle that string making, and it already does for some criteria though not for all. lazy could be insufficient because StopWhenAll and StopWhenAny unnecessarily force string building too.

@kellertuer this looks like a relatively simple improvement I could do soon-ish.

bvdmitri commented 3 months ago

Ah, using the get_reason function is even better, since then it is unnecessary to allocate even this empty string on creation.

kellertuer commented 3 months ago

Yeah I have not yet gotten around all string manifold optimisation tricks ;)

Thanks for reporting this. Moving the allocations to just get_reason is (I think) something we discussed already but have not yet gotten around to do everywhere. But I think only creating strings when using for reason is a good idea :)

PS: Those are by the way not error messages but more like human-readable information about why the solver stopped. It might actually more like report “Hooray! We made it!” (would not consider that an error 🤓)

PPS: And of course thanks for the kind works on the package :) Since Mateusz already has the idea realised in a few places I am sure we can improve this soon (the idea stems from a time, where I was not yet taking care of allocations too much – I hope I do better on newer code now)

kellertuer commented 2 months ago

We hopefully fixed this in the linked PR (version 0.4.65 to be registered later today). Thanks for reporting this!

If you have a cool example to show what you do on manifolds, feel free to open a discussion here or maybe even contribute an example to ManoptExamples.jl

bvdmitri commented 2 months ago

@kellertuer @mateuszbaran Thanks for improving this so quickly!

Yeah, we do cool stuff with your even cooler set of libraries. So what we effectively achieved is a "projection" algorithm of an arbitrary function (which should represent a log probability density function) to an arbitrary (yet compatible) member of the exponential family of distributions. E.g we can approximate an arbitrary f with the Gaussian distribution or Beta. This is done by optimizing the natural parameters of those distributions on their respective manifolds.

We achieved it with these libraries: https://github.com/ReactiveBayes/ExponentialFamilyManifolds.jl and https://github.com/ReactiveBayes/ExponentialFamilyProjection.jl which are based on https://github.com/ReactiveBayes/ExponentialFamily.jl and of course use a lot of manifolds stuff.

Eventually, this will be part of our larger project for fast Bayesian inference: RxInfer.jl. I think we can make an example once we integrate all of these parts with RxInfer, but technically it can also be a standalone example as here or here.

By the way, I am helping to organize the next JuliaCon in Eindhoven. If any of you plan to attend or would like to volunteer, I’d love to connect with you there in person.

kellertuer commented 2 months ago

Oh, that sounds super cool. If that is included in RxInfer.jl, we would love to get a note, we can surely link from Manopt.jl/Manifolds.jl to that package as a cool application!

I am also at JuliaCon, though I will talk about Quarto (see https://pretalx.com/juliacon2024/talk/JGT893/) and not about Manifolds/Manopt – sure we should then check to meet in person!

bvdmitri commented 2 months ago

Ah, cool, I know Patrick in person, the world is small :) see you on JuliaCon then!

kellertuer commented 2 months ago

I actually do not know him in person and we just talked on zoom once – but yeah, the world is still small. See you in Eindhoven :)