Add Diffeo abstraction #1904
Add Diffeo abstraction #1904
Conversation
…te_curves (incomplete) and univariate normal
…ith FiberBundle/QuotientMetric
…parameter in ElasticMetric
…random_point in DiscreteCurvesStartingAtOrigin
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
e2b4e2f to
8dd20f2
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1904 +/- ##
==========================================
+ Coverage 91.00% 91.47% +0.48%
==========================================
Files 220 222 +2
Lines 18868 18983 +115
==========================================
+ Hits 17168 17363 +195
+ Misses 1700 1620 -80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@luisfpereira Hi Luis, thank you for letting me know ! I just read the new classes and think is great! It unify the structure of the notions. Also the subclass for autodiff behavior is way more elegant ! |
6e54955 to
5681f0c
Compare
|
View / edit / reply to this conversation on ReviewNB alebrigant commented on 2023-11-27T12:15:38Z I would remove "(together with translation)" because it sounds like translation is not quotiented out here. |
| 6D40 | for n_times in [20]: | |
| data.extend([dict(n_points=n_points, n_times=n_times) for n_points in [1]]) | ||
| return self.generate_tests(data) | ||
| class AlignerCmpTestData(TestData): |
There was a problem hiding this comment.
What is "Cmp"? Is AlignedCmp a class in the codebase? Can you leave a docstring explaining why it deserves to have a spcial TestData?
There was a problem hiding this comment.
"Cmp" stands for comparison. This is a kind of test I've recently added that compares the outputs of two objects that do the same job. e.g. in this case it compares the outputs of the two algorithms we have for alignment.
I've implemented a similar comparison for metrics: if we have two different ways of implementing a given metric, it is very powerful to compare the results both provide as it would be very unlikely two independent ways of doing the same thing give the same results in random data.
tests/tests_geomstats/test_geometry/data/positive_lower_triangular_matrices.py
Show resolved
Hide resolved
|
|
||
| space.equip_with_group_action("reparametrizations") | ||
| space.equip_with_quotient_structure() | ||
| aligner = IterativeHorizontalGeodesic() |
There was a problem hiding this comment.
NIT: this class' name should probably end with Aligner, to match the logic of line 178.
Following the ideas started in #1863, this PR brings in the notion of
Diffeo.Additionally, it refactors
discrete_curves(done with @alebrigant). This happens together, because this module is heavily dependent on theDiffeoabstraction:SRVTransform, andFTransform.Design of
Diffeo(This part may interest you @ymontmarin.)
Most of it results from what was already implemented in
PullbackDiffeoMetric.In particular,
diffeomorphism, andinverse_diffeomorphismkeep the same signature, whiletangent_diffeomorphism, andinverse_tangent_diffeomorphismhave an additionalimage_pointorbase_pointdue to reasons specified in the docstrings of the abstract class.We've also decided to split the autodiff part in a new class called
AutodiffDiffeo. WhileDiffeois purely abstract,AutodiffDiffeo(a child ofDiffeo) brings in implementations fortangent_diffeomorphismandinverse_tangent_diffeomorphismgiven implementations ofdiffeomorphismandinverse_diffeomorphismthrough automatic differentiation. I think this split makes sense both from a conceptual and coding point of view, asAutodiffDiffeodefines several methods that are required almost exclusively for the automatic differentiation (e.g.jacobian_diffeomorphism) and are usually not considered outside of this scope. (This also simplifies implementations as the ones mentioned below)Moving these methods out of$M \to N$ in a diffeo from $N \to M$ . More interestingly, $M \to N \to P$ ) and creates a (composed) diffeo between the first and last manifold (e.g. $M \to P$ ). This may be useful to avoid creating unnecessary intermediate representations, while still keeping the possibility of using such representations without code duplication if wanted.
PullbackDiffeoMetricto a new abstraction (Diffeo) also allows to bring in new implementations that build on top of the new class. In particular,ReversedDiffeojust converts aDiffeofromComposedDiffeotakes several diffeos (e.g.Affected modules
While several modules were affected by this change, the main changes were done in
spd_matrices/hpd_matrices, anddiscrete_curves.spd_matrices/hpd_matrices(This part may interest you @YannCabanes, as
hpd_matriceshas changed similarly tospd_matrices.)We've created the diffeomorphisms
LogDiffeo,PowerDiffeo, andCholeskyMap. This hugely simplifiesSPDMatricesAPI. I've also transformed some methods into functions due to wide use (logmh,expm,powermh). The intention is to move them to the backend afterwards.Due to the new
LogDiffeo, we've implementedSPDLogEuclideanMetricas aPullbackDiffeoMetric.Due to the new
PowerDiffeo, we've removedpower_affine, andpower_euclideanfromSPDAffineMetric, andSPDEuclideanMetric, respectively, as this metric can be obtained withSPDPowerMetric(which takes advantage both fromPullbackDiffeoMetricandScalarProductMetric) with much simpler (and complete) code, without any performance degradation. For example,SPDEuclideanMetric.parallel_transportwould raise aNotImplementedErrorforpower_euclidean != 1, whereas now it works properly.discrete_curves(This part may interest you @MMalikT)
(with @alebrigant )
We've decided, for the moment, to focus only on
DiscreteCurvesStartingAtOrigin, which are discrete curves modulo translation.The method
projectiontakes aDiscreteCurveand removes translation and origin (@alebrigant, we may need to iterate on this, but for now, not having the origin in the point representation is much more convenient in order to take advantage of existing implementations, such asNFoldManifold). This method is widely used in the already existing notebooks to keep the previous behavior. Within the notebooks we've added back the origin before plotting.We've implemented
SRVTransformandFTransform. The corresponding pullback metrics (ElasticTranslationMetricandSRVTranslationMetric) simply inherit fromPullbackDiffeoMetricwithout overriding any code.For the quotienting by reparameterizations, we've divide the algorithms we have to align curves into two classes (
IterativeHorizontalGeodesic, andDynamicProgrammingAligner). This follows an idea we were already pursuing ingraph_spaceand deeply simplifies the implementation ofSRVTranslationReparametrizationBundle(we simply need to pass an aligner - or accept the default one) andSRVTranslationReparametrizationQuotientMetric(the implementation is independent of the algorithm). Both algorithms are still fragile and their implementations need to be improved.Several auxiliar methods were added to (conceptually) simplify the implementations, such as
forward_difference,centered_difference, and so on. A next step is to extend this notions to ambient manifolds other than the Euclidean space.We've fully removed
ClosedDiscreteCurvesbecause the implementation is not consistent with the changes. We need to revisit this afterwards.