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

New metrics correlation matrices #1695

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ythanwerdas
Copy link
Collaborator

@ythanwerdas ythanwerdas commented Oct 19, 2022

Checklist

  • My pull request has a clear and explanatory title.
  • If neccessary, my code is vectorized.
  • I have added apropriate unit tests.
  • I have made sure the code passes all unit tests. (refer to comment below)
  • My PR follows PEP8 guidelines. (refer to comment below)
  • My PR follows geomstats coding style and API.
  • My code is properly documented and I made sure the documentation renders properly. (Link)
  • I have linked to issues and PRs that are relevant to this PR.

Description

Issue

Additional context

Copy link
Collaborator

@luisfpereira luisfpereira left a comment

Choose a reason for hiding this comment

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

Thanks @ythanwerdas. it looks like this is going in the right direction.

Let's discuss together specially the comment on maybe creating StrictlyLowerTriangularMatrices instead.

geomstats/geometry/full_rank_correlation_matrices.py Outdated Show resolved Hide resolved
geomstats/geometry/full_rank_correlation_matrices.py Outdated Show resolved Hide resolved
geomstats/geometry/lower_triangular_matrices.py Outdated Show resolved Hide resolved
geomstats/geometry/lower_triangular_matrices.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 21, 2022

Codecov Report

Merging #1695 (3fa4117) into master (6d7b34a) will increase coverage by 4.86%.
The diff coverage is 29.17%.

@@            Coverage Diff             @@
##           master    #1695      +/-   ##
==========================================
+ Coverage   82.46%   87.31%   +4.86%     
==========================================
  Files         136      135       -1     
  Lines       13417    12922     -495     
==========================================
+ Hits        11063    11282     +219     
+ Misses       2354     1640     -714     
Flag Coverage Δ
autograd 87.31% <29.17%> (?)
pytorch ?

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

Impacted Files Coverage Δ
...omstats/geometry/full_rank_correlation_matrices.py 68.91% <20.00%> (-29.74%) ⬇️
geomstats/geometry/lower_triangular_matrices.py 70.91% <44.45%> (-25.75%) ⬇️

... and 54 files with indirect coverage changes

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

@luisfpereira luisfpereira force-pushed the new_metrics_correlation_matrices branch from 338b5b3 to 3fa4117 Compare July 12, 2023 11:49
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