Manopt.jl
Manopt.jl copied to clipboard
JuliaManifolds
Switch `storage` from a Dictionary to a NamedTuple
Recently, @mateuszbaran pointed out that it might be better to use a NamedTuple for the storage pf interims values (that can be shared between different Debugs and Stopping Criteria to avoid double computations/storage).
This seems to be a very good idea, just that the approach might not work for the Circle since then something like :x(soon :Iterate) will be a Float and can not be modified.
If we find a way to avoid this, the switch to a NamedTuple should not be that much work.
For Circle you can use fill(0.0) to represent points and tangent vectors, that's mutable.
How would that work? I would have to know whether the manifold has arrays or bits and depending on that initialise the fields differently as well as update them differently? (I think this would affect Circle(), Euclidean()?).
If you use fill(0.0) as initialization of optimization then it would work naturally. If you want 0.0 to be a valid, you have to add fill (or Ref). IIRC you already have AbstractEvaluationType that can be used for that? Basically if AllocatingEvaluation is used, then you should use fill or Ref for that NamedTuple.
The evaluation type is used for distinguishing grad_f(M,p) and grad_f!(M, X, p) in the function handles :)
I still do not understand what you mean. I want the user to not have to think about implementation details, so If a user wants to work in Circle with x0=0.0 as a start and hence points being floats that should just work (as it does currently as well).
So, the case when it wouldn't work is when tangent vectors are mutable and points are not? Well, a bit of a corner case but I see now. Anyway, for simplicity you can just always store Refs in NamedTuple.
No my main problem is any points/vectors that are not arrays but floats. Unless I overwrite the store command for these and Refany float?
I don't understand your problem now. Look here:
julia> f = (; a=Ref(10), b=Ref([11]))
(a = Base.RefValue{Int64}(10), b = Base.RefValue{Vector{Int64}}([11]))
julia> f.a[] = 12
12
julia> f
(a = Base.RefValue{Int64}(12), b = Base.RefValue{Vector{Int64}}([11]))
You can have a NamedTuple that stores references to floats, vectors, or anything else. You can change what these references point to (f.a[] = 12 updates the value). What's the problem with that?
So I would Ref(...)all my fields? Doesn't‘t that destroy the advantages again?
Yes, you can just Ref all fields. That would still be much better than a dictionary because NamedTuple is a type stable collection with very fast lookup.
Hm, sounds strange to me and seems a little hustle to implement it that way, what if the default is to not Ref – but have the special cases (I do a few anyways if manifolds is loaded) mentioned here to only Ref for a few Manifolds?
Sure, you can add a function like needs_ref(manifold, point) for determining if Ref is needed.
that sounds like a good idea :) Will try that as my next feature (ehem, well after Frank Wolfe and Times)
Ah it might be a little more tricky, since for example storing the cost (because evaluation it again might be too expensive) is probably a float independent of the manifold, so the ref has to be more flexible then just per manifold.
What if I just dispatch on isbits and ref all is bits that want to be stored?
Just wanted to add a note on NamedTuple. Each NamedTuple using different keys/fields does require compile time. So NamedTuple works well if it's use can be compiled once (or precompiled) and then used as the same type in a hot-loop computation. We recently had the problem that a NamedTuple had to constantly be compiled in regular usage, so we had to drop down to a tuple in our use-case (and changed code to be placement specific).
I think* Julia uses NamedTuple for keyword args in functions, so it is a good approach if the compiling is clear cut.
Thanks for the input. Indeed while the Tuple is used during the iterations to store “stuff” its types do not change (usually just the stored values in the arrays/ Refed numbers), so there will not be any recompilations necessary
Yes, using Dict vs NamedTuple is a tradeoff and NamedTuple is only going to be faster for very fast to compute objectives and gradients (my guess would be <1-10 us). So far this storage has never been close to a bottleneck for our use cases and things like #139 would have much more impact when implemented.
NamedTuples (with some extra stuff) are indeed used for keyword arguments.
For me it seems that this might for now be too much effort in implementation compared to the gain? At least for now it seems it to me might be quite some code rework
No, this one. CostGrad should be quite easy, just also providing an easy interface might be a little work.
So I am closing this for now, and we might want to reconsider this once the dictionary becomes a bottle-neck.