Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hyperbolic Riemann tensor #651

Merged
merged 10 commits into from
Sep 20, 2023
Merged

Conversation

hajg-ijk
Copy link
Member

This small PR implements the Riemann tensor for the Hyperbolic manifold, analogous to the one on the Sphere.

I know I should probably open an issue about this, but since it's related: I was wondering whether it would make sense to implement a sectional_curvature method for the manifolds where a riemann_tensor is available.

I also added a new textbook reference for the formula and myself to the list of contributors.

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #651 (6ae7cf0) into master (22dc01d) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   99.22%   99.20%   -0.02%     
==========================================
  Files         106      106              
  Lines       10430    10433       +3     
==========================================
+ Hits        10349    10350       +1     
- Misses         81       83       +2     
Files Changed Coverage Δ
src/manifolds/Hyperbolic.jl 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@mateuszbaran
Copy link
Member

Thanks, I think this is a good addition.

I know I should probably open an issue about this, but since it's related: I was wondering whether it would make sense to implement a sectional_curvature method for the manifolds where a riemann_tensor is available

What is computing sectional curvatures useful for? I'm generally trying to avoid adding functions that don't have a practical use. Note that most of MetricManifold stuff needs to be revisited so don't pay too much attention to it.

Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
@hajg-ijk
Copy link
Member Author

What is computing sectional curvatures useful for? I'm generally trying to avoid adding functions that don't have a practical use. Note that most of MetricManifold stuff needs to be revisited so don't pay too much attention to it.

Honestly, I only thought about it because I am using it in what I am doing at the moment, which is also the reason why I implemented the Riemann tensor for the hyperbolic space. I thought about it as just a utility function, much like some cost functions are exported by Manopt. However, I am not sure about all the possible use cases, and one might of course argue that it only takes a few lines to implement if needed.

@mateuszbaran
Copy link
Member

OK, sure, if you find it useful then we can add it to Manifolds.jl 🙂

@kellertuer
Copy link
Member

kellertuer commented Sep 19, 2023

I think that is a useful function that could be added (in a new PR), but should then also be properly documented what the interface is and what it does.

edit: Oh I misread the title, this function is indeed nice and useful here :) Thanks for documenting it carefully and directly also adding tests

Copy link
Member

@kellertuer kellertuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me (lgtm).

@hajg-ijk
Copy link
Member Author

I still don't understand why codecov/project fails. It laments an indirect change in the circle group, but when you look at it, it has no relation whatsoever to my changes.

@kellertuer
Copy link
Member

Nighlty is ok to fail and the rest looks good. all is fine.

@hajg-ijk hajg-ijk merged commit b5cdb3c into master Sep 20, 2023
26 of 30 checks passed
@kellertuer kellertuer deleted the hajg-ijk/hyperbolic-riemann-tensor branch May 4, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants