add segre and warped segre manifold

[sjacobsson]

No description provided.

[kellertuer]

Thanks for getting this started!
If you grant me access to your repository/fork, I can also push a few small things along the way.

A very short first look

• Signatures of functions in the doc string can also just be one line
• we usually added doc-strings to the non-inlace variant (e.g exp) while implementing the mutating one (e.g. exp!).
• doc strings would be great with a formula for most of the functions (inner, exp, log, norm,...)
• you already added the reference, you can refer to that in the doc string as well.
• I can help setting up the following as well
  • a separate page in the documentation for the new manifold
  • a bit for file structures see the other manifolds different files for different metrics
  • a first idea for testing

[sjacobsson]

Thanks for getting this started! If you grant me access to your repository/fork, I can also push a few small things along the way.

A very short first look

* Signatures of functions in the doc string can also just be one line

* we usually added doc-strings to the non-inlace variant (e.g `exp`) while implementing the mutating one (e.g. `exp!`).

* doc strings would be great with a formula for most of the functions (inner, exp, log, norm,...)

* you already added the reference, you can refer to that in the doc string as well.

* I can help setting up the following as well
  
  * a separate page in the documentation for the new manifold
  * a bit for file structures see the other manifolds different files for different metrics
  * a first idea for testing

Thanks, you should now be added as a collaborator to the fork.

I'll take a look at the doc stuff and try to conform to the rest of the repo. For the testing, I never figured out how to run the tests locally so I just commited my best guess. But once tests run, they should at least guarantee that the methods that Segre implements are consistent with each other.

[kellertuer]

No worries! Some parts of such a PR are a bit involved. I will both set up the tests for you and help you running them locally :)
On the other hand – tests are quite important to make sure the code continues to work as we expect, so we should definetly add them – also because we have a few checks here, that require to have a certain fraction (quite large part actually)of your code covered by tests.
But again, no worries, I am confident we will manage that together!

[sjacobsson]

Hi again,
Sorry, is there some way to view the documentation that I'm writing? I tried to run docs/make.jl but it ran into some errors.

[kellertuer]

HI,
an easy, but slow way, is to make sure Manifolds.jl compiles at least and then you can (now with the label added) render the docs here If you want.

Locally on your machine you have to set the package into development mode and then call docs/make.jl (either from shell as it is a shell script or including it because it is also a Julia script).

The development mode is maybe the tricky one (see https://pkgdocs.julialang.org/v1/managing-packages/#developing), running from shell should be a bit easier. We wrote the docs in its own environment so that should work fine.
It might be that running the make fiel for the docs itself errors, that happens whenever there is documented functions/types that are not yet added to the documentation. But as I said, when I find time I can create the page for your manifold and the metric.

If you can not get it to run you could post the error message here or we can check (after I found time) in a video call.

[sjacobsson]

Thanks, I'll try those things. No stress!

[mateuszbaran]

Thanks for the contribution! I see there is an error with an unrelated test, I will fix it soon in a separate PR.

[kellertuer]

I allowed myself to fix a few small things here

• merged master so the last fix from Mateusz also works here (not related to your code, but made CI fail)
• created a page in the docs.
• made sure docs work
• made Manifolds.jl compile – you had a few undeclared parameters. Not using them, one can even remove them
  • usually our field 𝔽 is the first parameter, I switched the order in Segre to make it first there as well since it confused me and makes a few function signatures nicer in the metric file.
  • I simplified said signatures – V could be removed in most places
  • removes all @assert within a function. We take the approach “input is correct/valid“ as far as possible – there is the ValidatioNManifold to activate those tests on a global level for a manifold. The advantage their is you can even choose the level of check – do you want to see an error for example or just an info
  • I removed the exp for now since that default we have implemented as well – if so we might want to write the corresponding allocate_result (https://github.com/JuliaManifolds/ManifoldsBase.jl/blob/7b52f311f3cc5bcd4671edb5479f798250c02876/src/exp_log_geo.jl#L16) – then much more functions also only need their !-variants.
• it would maybe be nicer to document is_point instead of `check_vector´ – it would also be great to have a bit more of a desciption there.
• checked that tests are included
  • they currently still fail, since you initialise some AlphaWarpedMetric? Was that renamed?
• ran JuliaFormatter (the format check here, run it with using JuliaFormatter; format(".") in the base folder of Manifolds.jl)
• started an entry in the news.
• Commented out tests for now, since then we get a first code coverage report.
• made them “standalone” they can just be run with include("test/manifolds/segre.jl") instead of running all tests – load other packages you need also there (and check they are in the extras of the project toml)

What would be great

• continue with the documentation, the formulae deyail in the beginning looks great (will read them carefully somewhen)
• check performance
• check test coverage
• add you to the contributors also in the zenodo info file for example
• I did not yet read any of the docs or code more closely but will definetly do that to learn about the new manifold as well

Besides all these comments – super cool that you got this started!

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 262 lines in your changes missing coverage. Please review.

Project coverage is 94.23%. Comparing base (784a6b9) to head (2b2da05).

Files with missing lines	Patch %	Lines
src/manifolds/Segre.jl	0.00%	175 Missing ⚠️
src/manifolds/SegreWarped.jl	0.00%	87 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #755      +/-   ##
==========================================
- Coverage   96.38%   94.23%   -2.16%     
==========================================
  Files         123      125       +2     
  Lines       11438    11700     +262     
==========================================
  Hits        11025    11025              
- Misses        413      675     +262     

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

[sjacobsson]

Thanks for fixing those things!

The problem is indeed a rename of AlphaWarpedMetric. I still don't know if the current name, WarpedMetric, is a good choice. The problem is that the euclidean metric is already a warped product metric, but then you can have a whole family of metrics based on your warping parameter and most of those are not euclidean...

Sorry, I still can't find where to view the documentation.

[kellertuer]

Ah it might be that as an external PR-opener this does not give you push rights, so the preview button does not appear, I have to check.
But still, just “out of the box” our docs/make.jl (either run via include on Julia REPL or as a shell script) should produce the docs as well. It activates its own environment and adds Manifolds.jl in development mode from the parent folder, so it should always generate the docs of the state you have on your hard drive.

For the name, try to find a name that fits well. For me WarpedMetric is fine but maybe a bit too generic (and the alpha does not help much then). Still, if you favour WarpedMetric as the type name for the metric, go for it :)

[sjacobsson]

I shall meditate on the name for today and tomorrow, otherwise let's just go with WarpedMetric.

I also tried running make.jl but I get the following error: ERROR: LoadError: 'tutorials/getstarted.md' is not an existing page!
Here is the full output:

simonj@beltrami:~/Workspace/Manifolds.jl$ julia
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using Manifolds

help?> Segre
search: Segre NestedReplacingPowerRepresentation base_group isinteger HeisenbergGroup SymplecticGrassmann

      \mathcal{S} = \operatorname{Seg}(𝔽^{n_1} \times \dots \times 𝔽^{n_d})

  is the space of rank-one tensors in 𝔽^{n_1} \otimes \dots \otimes 𝔽^{n_d}.

  When 𝔽 = ℝ, the Segre manifold is represented as

      \mathcal{S} \sim ℝ^{+} \times S^{n_1 - 1} \times \dots \times S^{n_d - 1}.

  This is a local diffeomorphism, and the manifold is a locally a warped product
  (https://en.wikipedia.org/wiki/Warped_product) of ℝ^{+} and S^{n_1 - 1} \times \dots \times S^{n_d -
  1}. The tuple (n_1, \dots, n_d) is called the valence of the manifold.

  The geometry is summarized in JacobssonSwijsenVandervekenVannieuwenhoven:2024 (@cite).

  Constructor
  ≡≡≡≡≡≡≡≡≡≡≡≡≡

  Segre(valence::NTuple{V, Int}; field::AbstractNumbers=ℝ)

  Generate a valence V Segre manifold.

julia> include("docs/make.jl")
  Activating project at `~/Workspace/Manifolds.jl/docs`
   Resolving package versions...
    Updating `~/Workspace/Manifolds.jl/docs/Project.toml`
  [1cead3c2] ~ Manifolds v0.10.4 `..` ⇒ v0.10.4 `~/Workspace/Manifolds.jl`
    Updating `~/Workspace/Manifolds.jl/docs/Manifest.toml`
  [1cead3c2] ~ Manifolds v0.10.4 `..` ⇒ v0.10.4 `~/Workspace/Manifolds.jl`
  No Changes to `~/Workspace/Manifolds.jl/docs/Project.toml`
  No Changes to `~/Workspace/Manifolds.jl/docs/Manifest.toml`
[ Info: Precompiling ManifoldsRecipesBaseExt [37da849e-34ab-54fd-a5a4-b22599bd6cb0]
    CondaPkg Found dependencies: /home/simonj/Workspace/Manifolds.jl/docs/CondaPkg.toml
    CondaPkg Found dependencies: /home/simonj/.julia/packages/PythonCall/Nr75f/CondaPkg.toml
    CondaPkg Found dependencies: /home/simonj/.julia/packages/PythonPlot/469aA/CondaPkg.toml
    CondaPkg Dependencies already up to date
[ Info: Precompiling ManifoldsRecursiveArrayToolsExt [e29539f8-2f71-57d7-85ce-957fb5efd4f1]
[ Info: Precompiling ManifoldsOrdinaryDiffEqExt [95431769-3d64-508c-9576-79148c2c82de]
[ Info: Precompiling ManifoldsBoundaryValueDiffEqExt [eb713886-0b96-5c41-a09d-5e8967e02f7d]
[ Info: Precompiling ManifoldsNLsolveExt [b22937d9-c54a-59ec-962e-7448ad3eacb1]
[ Info: Precompiling ManifoldsOrdinaryDiffEqDiffEqCallbacksExt [f484757d-caa2-58a4-a95c-7868e5350983]
[ Info: SetupBuildDirectory: setting up build directory.
ERROR: LoadError: 'tutorials/getstarted.md' is not an existing page!
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] walk_navpages(visible::Bool, title::String, src::String, children::Vector{Any}, parent::Documenter.NavNode, doc::Documenter.Document)
    @ Documenter ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:185
  [3] walk_navpages
    @ ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:200 [inlined]
  [4] walk_navpages
    @ ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:202 [inlined]
  [5] #81
    @ ./none:0 [inlined]
  [6] iterate
    @ ./generator.jl:47 [inlined]
  [7] collect(itr::Base.Generator{Vector{Pair{String, String}}, Documenter.var"#81#82"{Documenter.NavNode, Documenter.Document}})
    @ Base ./array.jl:782
  [8] walk_navpages
    @ ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:203 [inlined]
  [9] walk_navpages
    @ ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:190 [inlined]
 [10] walk_navpages(title::String, children::Vector{Pair{String, String}}, parent::Nothing, doc::Documenter.Document)
    @ Documenter ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:199
 [11] walk_navpages(p::Pair{String, Any}, parent::Nothing, doc::Documenter.Document)
    @ Documenter ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:202
 [12] (::Documenter.var"#81#82"{Nothing, Documenter.Document})(p::Pair{String, Any})
    @ Documenter ./none:0
 [13] iterate
    @ ./generator.jl:47 [inlined]
 [14] collect_to!(dest::Vector{Documenter.NavNode}, itr::Base.Generator{Vector{Any}, Documenter.var"#81#82"{Nothing, Documenter.Document}}, offs::Int64, st::Int64)
    @ Base ./array.jl:840
 [15] collect_to_with_first!
    @ ./array.jl:818 [inlined]
 [16] collect(itr::Base.Generator{Vector{Any}, Documenter.var"#81#82"{Nothing, Documenter.Document}})
    @ Base ./array.jl:792
 [17] walk_navpages
    @ ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:203 [inlined]
 [18] runner(#unused#::Type{Documenter.Builder.SetupBuildDirectory}, doc::Documenter.Document)
    @ Documenter ~/.julia/packages/Documenter/C1XEF/src/builder_pipeline.jl:133
 [19] dispatch(#unused#::Type{Documenter.Builder.DocumentPipeline}, x::Documenter.Document)
    @ Documenter.Selectors ~/.julia/packages/Documenter/C1XEF/src/utilities/Selectors.jl:170
 [20] #89
    @ ~/.julia/packages/Documenter/C1XEF/src/makedocs.jl:272 [inlined]
 [21] withenv(::Documenter.var"#89#91"{Documenter.Document}, ::Pair{String, Nothing}, ::Vararg{Pair{String, Nothing}})
    @ Base ./env.jl:197
 [22] #88
    @ ~/.julia/packages/Documenter/C1XEF/src/makedocs.jl:271 [inlined]
 [23] cd(f::Documenter.var"#88#90"{Documenter.Document}, dir::String)
    @ Base.Filesystem ./file.jl:112
 [24] #makedocs#87
    @ ~/.julia/packages/Documenter/C1XEF/src/makedocs.jl:271 [inlined]
 [25] top-level scope
    @ ~/Workspace/Manifolds.jl/docs/make.jl:95
 [26] include(fname::String)
    @ Base.MainInclude ./client.jl:478
 [27] top-level scope
    @ REPL[3]:1
in expression starting at /home/simonj/Workspace/Manifolds.jl/docs/make.jl:95

julia> 

[kellertuer]

Oh, sorry! I forgot about the (bit involved) Quarto examples. You either have to install Quarto (and add --quarto) to render them or pass --exclude-tutorials to the arguments, and yeah sorry that means for now it does not just work by include, since that both are command line args.

Sorry forgot about those nitty gritty details. And sure, for best of cases we should come up with something that “just works”, but for now rendering Quarto with pure Julia is still not yet there (and we would depend on that).
So for now you would have to call it from terminal then, sorry. (in your case as docs/make.jl --exclude-tutorials)

[sjacobsson]

No worries! Sorry for bothering you when you said you were busy, I just wanted to be able to see the docs so that I could work on them in the meantime.

I get the same error again with docs/make.jl --exclude-tutorials :(
When I look in make.jl there is an if "--quarto" ∈ ARGS, but I don't see any lines with --exclude-tutorials.

[kellertuer]

Ah, hm, yes. That comes from “I am busy but I can answer this in 2 minutes so let's just do it”-answers (mine, not yours).

What I wrote does hold for Manopt.jl, but not yet for Manifolds.jl (but then we should introduce this soon!) – so then, sorry, you would have to run first with --quarto to generate the files Documenter complains about and for that you have to install quarto,...
so maybe – you were right from the start – you can currently only render the docs with a bit of effort locally.
But since Manopt.jl has a solution for that I will transfer it here as soon as time permits.

[sjacobsson]

I got it to work now by commenting out lines 109-114 in make.jl:

            "🚀 Get Started with `Manifolds.jl`" => "tutorials/getstarted.md",
            "work in charts" => "tutorials/working-in-charts.md",
            "perform Hand gesture analysis" => "tutorials/hand-gestures.md",
            "integrate on manifolds and handle probability densities" => "tutorials/integration.md",
            "explore curvature without coordinates" => "tutorials/exploring-curvature.md",
            "work with groups" => "tutorials/groups.md",

Thanks for the help! Now I'll try to write a nice documentation page like the ones for the Stiefel etc :)

[kellertuer]

Ah, yeah, the ARG --exclude-tutorials in principle does the same thing ;) Great that you got it to work, just remember not to commit the commenting out.

I also got you started already on such a page, see docs/src/manifolds/segre.md, which automatically includes all docs from both files you write – but sure a bit more structure like in the Stiefel page or the SPD case are also nice to have, especially with a default metric and a second (the warped) one.

[sjacobsson]

I see the segre.md that you added, very nice! I also see some texing errors that I made along the way. Thanks!