Use sectional_curvature from ManifoldsBase

[mateuszbaran]

Ideally it should use sectional_curvature_max but it's not generally implemented in Manifolds.jl yet.

I've also removed the unused k_min keyword argument.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.73%. Comparing base (15194b0) to head (94122a8).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #365      +/-   ##
==========================================
- Coverage   99.73%   99.73%   -0.01%     
==========================================
  Files          73       73              
  Lines        6822     6819       -3     
==========================================
- Hits         6804     6801       -3     
  Misses         18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mateuszbaran]

Documenter.jl thinks ManifoldsBase.jl 0.15.8 doesn't exist but CI tests use it without issue... weird.

[kellertuer]

Maybe the docs need a small bump? But I wanted to check real quick the interlinks anyways, now that ManifoldsBase has run on Documenter 1.3 and has a first such index to use for the inter-docs :)

[kellertuer]

This looks good to me. Maybe we can check whether the docs are ok when merged with #366 .

[kellertuer]

@hajg-ijk do you maybe want to take a look as well?

[hajg-ijk]

Looks good to me, too.
The only note here is that ζ_1 (which is not used anywhere at the moment) in theory takes in the lower bound on the curvature k_min as an input.

[mateuszbaran]

Now we have a different error:

┌ Error: On "src/solvers/convex_bundle_method.md", cannot resolve external link: Cannot find "`ManifoldsBase.sectional_curvature`" in any InterLinks inventory: ["ManifoldsBase"]
│   node =
│    @ast MarkdownAST.Link("@extref", "") do
│      MarkdownAST.Code("ManifoldsBase.sectional_curvature")
│    end
│    
└ @ DocumenterInterLinks ~/.julia/packages/DocumenterInterLinks/jzyZp/src/expand_extrefs.jl:93

I thought that would work, like https://github.com/JuliaManifolds/Manopt.jl/pull/366/files#diff-6b82ea9e272b78f9938c76cb57acca981b149e4c4a319e5be73944701d1d693bR29 ?

[kellertuer]

Had to take a look, the main problem is, that the method has 3 signatures (abstract manifold, power, product) so you have to specify the signature. That is what I fixed in my commit.

Might look a bit complicated, but I also have no idea how InterLink could make that better.

[kellertuer]

In other places I do not use the ManifoldsBase prefix in the link text – should we keep that here, what do you think?

[mateuszbaran]

I see, it's different for types and methods. I think we can remove the prefix then.

[kellertuer]

One tipp that Michael also gave is, to using DocumenterInterLinks and then load the

links = InterLinks(
    "ManifoldsBase" => ("https://juliamanifolds.github.io/ManifoldsBase.jl/stable/")
)

then you can query

julia> links["ManifoldsBase"]("sectional_curvature-")
3-element Vector{DocInventories.InventoryItem}:
 InventoryItem(":jl:method:`ManifoldsBase.sectional_curvature-Tuple{AbstractManifold, Any, Any, Any}`" => "functions/#ManifoldsBase.sectional_curvature-Tuple%7BAbstractManifold%2C%20Any%2C%20Any%2C%20Any%7D")
 InventoryItem(":jl:method:`ManifoldsBase.sectional_curvature-Tuple{AbstractPowerManifold, Any, Any, Any}`" => "metamanifolds/#ManifoldsBase.sectional_curvature-Tuple%7BAbstractPowerManifold%2C%20Any%2C%20Any%2C%20Any%7D")
 InventoryItem(":jl:method:`ManifoldsBase.sectional_curvature-Tuple{ProductManifold, Any, Any, Any}`" => "metamanifolds/#ManifoldsBase.sectional_curvature-Tuple%7BProductManifold%2C%20Any%2C%20Any%2C%20Any%7D")

and everything after the method: is what you copy (you might guess this can also indicate types or sections or such)

[mateuszbaran]

Thanks, that's a good tip.

[kellertuer]

I see, it's different for types and methods.

Yes, since types are uniquely identified by their name, methods need the full signature with parameters, but then the list from above is helpful.