clean test_spd_matrices with parametrization #1263
Conversation
4a62036 to
6e222c7
Compare
tests/conftest.py
Outdated
| Parameters | ||
| ---------- | ||
| space : cls | ||
| Manifold class that upon which metric is present. |
There was a problem hiding this comment.
does new doc string work? not sure If I completely understand what I mean
There was a problem hiding this comment.
Excellent !! Such a relief to have better tests now 🙌 We will finally be able to properly evaluate the performances.
High-level comments:
-
n_points,num_samples,num_points,n_samplesare used inconsistently: let's usen_samplesfor now because (i) this is what is used in Geomstats codebase, (ii) this follows Sklearn's API see https://github.com/scikit-learn/scikit-learn/blob/49043fc769d0affc92e3641d2d5f8f8de2421611/sklearn/cluster/_agglomerative.py#L70 -
Some tests have disappeared (I think..?), mostly some
_vectorizationtests for which I did not find batch data in the_datamethods.
Otherwise it is perfect (minor comments are inline)! Can't wait to merge!
|
Thanks! I'll have a look once the tests pass 🤞 |
|
Those are fiber bundles tests. |
|
Ooooh ok :( |
I am adding some batch data |
There was a problem hiding this comment.
Thanks for addressing some comments! 🙌
Other comments from the previous code review have not been addressed (e.g. on num_points, n_points, n_samples, num_samples; or on no commented code going to the master branch, deep source is failing on this too; or on batch data): I'll let you have a look? Thanks!
|
|
||
| def random_point_and_belongs_data(self): | ||
| smoke_data = [ | ||
| dict(n=1, n_points=1), |
|
|
||
| def differential_gram_belongs_data(self): | ||
| ns = [1, 2, 2, 3, 10, 5, 100, 1000] | ||
| num_points = [1, 1, 2, 50, 1000, 100, 10, 5] |
|
|
||
| def inverse_differential_gram_belongs_data(self): | ||
| ns = [1, 2, 2, 3, 10, 15, 100, 1000] | ||
| num_points = [1, 1, 2, 200, 1000, 100, 10, 5] |
| def test_random_point_and_belongs(self, n, n_points): | ||
| space_n = self.space(n) | ||
| self.assertAllClose( | ||
| gs.all(space_n.belongs(space_n.random_point(n_points))), True |
|
|
||
| def random_point_data(self): | ||
| smoke_data = [ | ||
| dict(n=1, num_points=1), |
|
Professor @ninamiolane, you can check it now (hopefully I didn't miss anything) and tests that are failing are from fiber bundles methods |
clean test_spd_matrices