JuliaManifolds / Manifolds.jl

Manifolds.jl provides a library of manifolds aiming for an easy-to-use and fast implementation.
https://juliamanifolds.github.io/Manifolds.jl
MIT License
368 stars 53 forks source link

Add more SE2 and SE3 performance dispatches #660

Closed Affie closed 11 months ago

Affie commented 11 months ago
Affie commented 11 months ago

@mateuszbaran would you mind helping to implement the performance dispatches I tried to do in this PR? I thought I could help, but I'm still figuring out Manifolds.jl

mateuszbaran commented 11 months ago

Sure, I will check it out.

codecov[bot] commented 11 months ago

Codecov Report

Merging #660 (9ca5550) into master (106a8f3) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #660   +/-   ##
=======================================
  Coverage   99.22%   99.22%           
=======================================
  Files         106      106           
  Lines       10481    10488    +7     
=======================================
+ Hits        10400    10407    +7     
  Misses         81       81           
Files Coverage Δ
src/groups/special_euclidean.jl 99.63% <100.00%> (+<0.01%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

Affie commented 11 months ago

Not only SVector for coordinates: we take the coordinates from a larger vector in some places.

It looks like the view causes allocations in some instances, so perhaps your original SA is better. Will it be a problem with the coordinates c::AbstractVector and not s::SArray. What I tested so far looks like it doesn't have allocations.

mateuszbaran commented 11 months ago

It looks like the view causes allocations in some instances, so perhaps your original SA is better. Will it be a problem with the coordinates c::AbstractVector and not s::SArray. What I tested so far looks like it doesn't have allocations.

SA should not be a problem.