Simplify computations of the FrechetMean for the ElasticMetric #1592
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1592 +/- ##
==========================================
+ Coverage 88.65% 89.70% +1.06%
==========================================
Files 107 107
Lines 10694 10669 -25
==========================================
+ Hits 9480 9570 +90
+ Misses 1214 1099 -115
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
The failing tests are not related to this PR. |
| @@ -464,7 +465,7 @@ def __init__( | |||
| " equipped with a metric." | |||
| ) | |||
| self.ambient_manifold = ambient_manifold | |||
| self.l2_metric = L2CurvesMetric(ambient_manifold=ambient_manifold) | |||
| self.l2_curve_metric = L2CurvesMetric(ambient_manifold=ambient_manifold) | |||
There was a problem hiding this comment.
Maybe rename to l2_curves_metric for coherence, or L2CurvesMetric to L2CurveMetric?
| @@ -670,7 +671,7 @@ def f_transform_inverse(self, f, starting_point): | |||
| ) | |||
| if gs.ndim(f) != gs.ndim(starting_point): | |||
| starting_point = gs.to_ndarray(starting_point, to_ndim=f.ndim, axis=-2) | |||
| self.l2_metric | |||
| self.l2_curve_metric | |||
There was a problem hiding this comment.
What does this line do ? I don't see it in the current version of the master branch. For some reason, the current version used to compare changes in this PR does not seem to correspond to the current version of the master branch.
There was a problem hiding this comment.
This was a left over,thanks for catching it
| distance = 2 * self.b * gs.arccos(gs.clip(cosine, -1, 1)) | ||
| else: | ||
| distance = l2_dist(f_1, f_2) | ||
| # elastic_metric.dist |
This PR aimed to simplify the computation of the FrechetMean for the ElasticMetric, and ended up solving a series of bugs on elastic curves.
The method for the FrechetMean on elastic curves works as follows:
This PR also:
Checklist
Description
Issue
Additional context