Update MeanMaxPPE to a point on manifold

[Affie]

TODO serialization of MeanMaxPPE

[codecov[bot]]

Codecov Report

Merging #788 (49426e5) into master (54c1289) will increase coverage by 5.64%. The diff coverage is 20.00%.

@@            Coverage Diff             @@
##           master     #788      +/-   ##
==========================================
+ Coverage   48.82%   54.47%   +5.64%     
==========================================
  Files          31       31              
  Lines        2597     2651      +54     
==========================================
+ Hits         1268     1444     +176     
+ Misses       1329     1207     -122     

Impacted Files	Coverage Δ
src/services/CompareUtils.jl	19.35% <0.00%> (-0.78%)	:arrow_down:
src/services/Serialization.jl	70.35% <0.00%> (ø)
src/entities/DFGVariable.jl	78.94% <100.00%> (ø)
src/Deprecated.jl	16.92% <0.00%> (-17.56%)	:arrow_down:
src/DataBlobs/services/AbstractDataEntries.jl	96.77% <0.00%> (+0.34%)	:arrow_up:
src/services/DFGVariable.jl	71.84% <0.00%> (+3.21%)	:arrow_up:
src/LightDFG/FactorGraphs/BiMaps.jl	90.90% <0.00%> (+3.40%)	:arrow_up:
src/services/DFGFactor.jl	86.36% <0.00%> (+4.54%)	:arrow_up:
src/services/AbstractDFG.jl	82.95% <0.00%> (+10.57%)	:arrow_up:
src/DataBlobs/services/BlobStores.jl	81.63% <0.00%> (+22.25%)	:arrow_up:
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 54c1289...49426e5. Read the comment docs.

[Affie]

What is the reason to add the variable type to ppe? won't it always be attached to a variable that has the type?

Also: why is the type included as a field and a type parameter? Is a type parameter not enough. Or a field only, but I don't know if ::Type is bad for performance.

[dehann]

What is the reason to add the variable type to ppe? won't it always be attached to a variable that has the type?

Previous conversation where we decided to duplicate on purpose, so that when a user gets a PPE in non-Julia world, they can back out what the values mean. PPE and VND might not always travel together.

Also: why is the type included as a field and a type parameter? Is a type parameter not enough. Or a field only,

Only should be good enough, lets perhaps track that in an issue and figure that decision out when the manifolds and IIF 1010 work simmers down a bit?

but I don't know if ::Type is bad for performance.

I also don't know. The first thing that comes to mind for me is if abstract types and type inference is needed when working with types. The second thing that comes to mind is that Base already uses the idea of Type for converters, eg convert(;:Type, obj) so it cannot be that bad for performance?

[dehann]

This is sort of coming up, adding xref here for future design:

• https://github.com/JuliaRobotics/IncrementalInference.jl/issues/1507