-
Notifications
You must be signed in to change notification settings - Fork 13
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
Should a Lazy wrapper for Manifolds.jl exist? #450
Comments
Option A Pose as is
I wouldn't implement this without concrete types on p1 and p2. The second reason, although not really that important, is it makes it slightly easier for me to use. function PosePose(ⁱmⱼ::SE{N}, ʷxᵢ::SE{N}, ʷxⱼ::SE{N}) where N
ʷT̂ⱼ = ʷxᵢ ∘ ⁱmⱼ
ʲT̂ⱼ = ʷxⱼ \ ʷT̂ⱼ
X = log(ʲT̂ⱼ)
return vee(X)
end The equivalent will be something like this to implement Pose2 and Pose3: function Pose2Pose2(ⁱmⱼ, ʷxᵢ, ʷxⱼ)
M = SpecialEuclidean(2)
ʷT̂ⱼ = compose(M, ʷxᵢ, ⁱmⱼ)
ʲT̂ⱼ = compose(M, inv(M, ʷxⱼ), ʷT̂ⱼ)
X = log(M, identity(M,ʲT̂ⱼ), ʲT̂ⱼ)
return vee(M, identity(M,ʲT̂ⱼ), X)
end
function Pose3Pose3(ⁱmⱼ, ʷxᵢ, ʷxⱼ)
M = SpecialEuclidean(3)
ʷT̂ⱼ = compose(M, ʷxᵢ, ⁱmⱼ)
ʲT̂ⱼ = compose(M, inv(M, ʷxⱼ), ʷT̂ⱼ)
X = log(M, identity(M,ʲT̂ⱼ), ʲT̂ⱼ)
return vee(M, identity(M,ʲT̂ⱼ), X)
end A third reason is the constructors that are currently in TransformUtils. eg. That being said, I'm also not completely convinced either and AMP is likely the place that will determine what is needed. I would also like to explore the possibility of the variable actually being the type. see next comment |
Option B - struct Pose{M <: Manifolds.AbstractGroupManifold, T}
value::T
end mutable struct VariableNodeData{T<:InferenceVariable}#, P, B}
"Vector of a point on the manifold, Eg Pose{SE2}(x,y,θ)"
val::Vector{T}
... Option C - abstract type InferenceVariable{M<:AbstractManifold, T} end
struct Pose{M<:SpecialEuclidean, T} <: InferenceVariable{M, T} end
const Pose2 = Pose{SpecialEuclidean{2}, ProductRepr{Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}}
const Pose3 = Pose{SpecialEuclidean{3}, ProductRepr{Tuple{SVector{3, Float64}, SMatrix{3, 3, Float64, 9}}}}
var_type = Pose{SpecialEuclidean{2}, SVector{3,Float64}}()
getPointType(::Type{<:InferenceVariable{M, T}}) where {M,T} = T
getManifoldType(::Type{<:InferenceVariable{M, T}}) where {M,T} = M
vnd.val::Vector{getPointType(var_type)}() |
Hi Johan, thanks for the write-up, I have many thoughts and will respond later today or tomorrow once I get the ideas organized. I think there is an Option D which I favor, but let me go make sure first. Thanks again for the write-up! xref
|
Rabbit hole: To capture it somewhere: These are based on f:M→ℝ #manifolds point to manifold point factor _d uses distance function
function PointPoint_d(m, p, q)
q̂ = p ∘ m
return distance(q, q̂)
end
# manifold prior factor _d uses distance function
function Prior_d(meas, p)
return distance(meas, p)
end or not using LazyLieManifolds. function PointPoint_d(M::AbstractGroupManifold, m, p, q)
q̂ = compose(M, p, m)
return distance(M, q, q̂)
end
Pose2Pose2(m, p, q) = PointPoint_d(SpecialEuclidean(2), m, p, q)
Pose3Pose3(m, p, q) = PointPoint_d(SpecialEuclidean(3), m, p, q)
Point2Point2(m, p, q) = PointPoint_d(TranslationGroup(2), m, p, q)
Point3Point3(m, p, q) = PointPoint_d(TranslationGroup(3), m, p, q) |
Option DKeep getDimension(::Manifold)
getManifold(::InferenceVariable) This resolves through dispatch how "Manifold type" information is retrieved (i.e. For serialization, we expressly decided against any value information inside Serialization of full manifold types as part of variable types for Option C is also hard, however, an easy getout would be to just use the @Affie and I spoke on the phone, and a workable solution for resolving "point type" information is to include this via:
i.e. both "manifold type" and "point type" information is provided by the user when creating our "variable type". Users can then more easily create as many "variable types" as they want in the Most important though, is to NOT require other new abstract definitions or wrapper types for existing Manifolds types. Once we have nonparametric and parametric computations fully consolidated on Manifolds.jl operations (i.e. deprecate TransformUtils.jl), then we can as a later step return with wrappers to help (e.g. @defVariable Pose3_EZ Manifolds.SpecialEuclidean(3) LazyLieSE2Point{Float64}
# or even do a new macro
@easyDefVariable Pose3_EZ LazyLieSE2Point{Float64} But this is only a later step AFTER IIF has been transitioned to Manifolds without adding new types and abstractions. I'm trying to keep both maximum flexibility and lessons learned inside Manifolds.jl available here (preserving versatility, utility and unity in Julia ecosystem), while also minimizing the changes in IIF/RoME. I understand Options A,B,C above as "already assume wrappers are the way to go". I originally thought we should go that way, but now I'm more convinced that this Option D is the necessary first step -- otherwise we are going to take on way too much work at once, and or stir dissonance among "manifold types".
|
In summary: We are going with option D, which is option A but with a Lazy wrapper only implemented at a later stage if needed. In options B and C, the lazy wrapper was implemented in the I'm pretty sure a lazy wrapper (or a place to extend Manifolds.jl) will come in handy later. |
The decision for IIF v0.25 was that LazyWrapper is a convenience, and IIF / RoME should foremost first support the same thing Manifolds.jl does, so users can mix and match the ecosystem packages. If we are able to build a convenient lazy wrapper which does not inhibit this earlier requirement, we can slowly work in that direction. At time of writing, I'm less enthusiastic about a LazyWrapper (even though I originally helped promote the idea). At present I am more in support of the design philosophy Manifolds.jl followed. Linking this text to High Level Design Wiki: https://github.com/JuliaRobotics/Caesar.jl/wiki/High-Level-Requirements |
Hi @Affie, i'm having doubts about this whole approach. I'm now thinking we should not be building types like SE and se at all.
I think we should drastically reduce the amount of duplication with Manifolds.jl and just simply use
VND.val::Vector{ProductRepr...}
and implement just RoME.oplus(::Pose3Pose3, p1,p2) which follows the same style as Manifolds.jl. That way we avoid the exp/retract issue we had in 366
I really want to avoid duplicating types, abstracts and API design from Manifolds. I'm actually thinking we should not implement a Lazy wrapper at all for the next couple of weeks and simply add the oplus functions in RoME until we have a better idea of what does and doesn't work. All we doing is dispatch against func(::AbstractFactor,...). E.g.
which is easily overloaded for
Originally posted by @dehann in dehann/TransformUtils.jl#45 (comment)
The text was updated successfully, but these errors were encountered: