8000 Add new flat Matrices metrics by luisfpereira · Pull Request #1947 · geomstats/geomstats · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add new flat Matrices metrics #1947

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

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

luisfpereira
Copy link
Collaborator

This PR builds on top of #1946 to add new metric matrices for Matrices (and other matrix vector spaces). Explicitly, we add MatricesDiagMetric. With the addition of diffeomorphisms to the EuclideanSpace, we implicitly add the ability to easily implement a PullbackDiffeoMetric from an Euclidean space equipped with a non-standard EuclideanMetric.

Next PR will show application of these concepts.

NB: merging this PR before #1946 also merges #1946.

Copy link
codecov bot commented Feb 5, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (3222c7d) 91.37% compared to head (53ebf0c) 91.48%.

❗ Current head 53ebf0c differs from pull request most recent head 3a0c730. Consider uploading reports for the commit 3a0c730 to get more accurate results

Files Patch % Lines
geomstats/geometry/matrices.py 80.44% 9 Missing ⚠️
geomstats/vectorization.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1947      +/-   ##
==========================================
+ Coverage   91.37%   91.48%   +0.12%     
==========================================
  Files         146      143       -3     
  Lines       13450    13009     -441     
==========================================
- Hits        12288    11900     -388     
+ Misses       1162     1109      -53     
Flag Coverage Δ
autograd 87.67% <84.85%> (?)
numpy 90.34% <84.85%> (+1.02%) ⬆️
pytorch ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Collaborator
@ninamiolane ninamiolane left a comment

Choose a reason for hiding this comment

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

Most docstrings are missing parameter descriptions. I'll review after they are added, because it'll be easier to read and understand the code with them.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@luisfpereira luisfpereira force-pushed the matrices-metrics branch 2 times, most recently from 4c1ec0c to d3fdf88 Compare February 12, 2024 14:56
@luisfpereira
Copy link
Collaborator Author

@ninamiolane, docstrings updated. Could you please review now?

@luisfpereira luisfpereira merged commit 4bd9677 into geomstats:main Feb 19, 2024
@luisfpereira luisfpereira deleted the matrices-metrics branch February 19, 2024 11:04
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.

2 participants
0