JuliaManifolds / ManifoldsBase.jl

Basic interface for manifolds in Julia
https://juliamanifolds.github.io/ManifoldsBase.jl/
MIT License
87 stars 8 forks source link

Add interface for optionally static size #160

Closed mateuszbaran closed 1 year ago

mateuszbaran commented 1 year ago

Basic infrastructure for solving https://github.com/JuliaManifolds/Manifolds.jl/issues/625 .

codecov[bot] commented 1 year ago

Codecov Report

Merging #160 (c35a1be) into master (71f85b6) will decrease coverage by 0.12%. The diff coverage is 100.00%.

:exclamation: Current head c35a1be differs from pull request most recent head 9d68f5b. Consider uploading reports for the commit 9d68f5b to get more accurate results

@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
- Coverage   99.96%   99.84%   -0.12%     
==========================================
  Files          19       20       +1     
  Lines        2539     2553      +14     
==========================================
+ Hits         2538     2549      +11     
- Misses          1        4       +3     
Files Changed Coverage Δ
src/DefaultManifold.jl 100.00% <100.00%> (ø)
src/maintypes.jl 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

mateuszbaran commented 1 year ago

While I think this is quite technical and should not concern beginners, it might be nice for advanced usages. A small benchmark illustrating when which is nicer might be nice as well (if that can be well-illustrated)

Yes, it's definitely not for beginners. Benchmarks would be hard at the moment because all good examples live in Manifolds.jl. I think rotation matrices would make a great example for a benchmark.

kellertuer commented 1 year ago

Sure let's just keep that a bit in mind to explain this feature with a benchmark somewhen. I think I see advantages of both and disadvantages, but having a small tutorial on that would be great – sure probably only in Manifolds.jl

mateuszbaran commented 1 year ago

Sure, I will add a clear explanation with an example in the Manifolds.jl PR that converts key manifolds to AbstractSize.