geomstats / geomstats

Computations and statistics on manifolds with geometric structures.
http://geomstats.ai
MIT License
1.24k stars 249 forks source link

Add unit tests for backend functions #304

Open nkoep opened 4 years ago

nkoep commented 4 years ago

The backend functions are currently not tested individually in isolation. This means that bugs in the backend abstractions are only spotted by unit tests if some of the more involved tests actually use a particular backend function. This would also resolve https://github.com/geomstats/geomstats/issues/303.

opeltre commented 4 years ago

[torch] : gs.expm does not seem properly vectorised, neither gs.matmul

opeltre commented 4 years ago

expected:

gs.matmul(a, gs.array([b, c]))
>>> [ab, ac]
opeltre commented 4 years ago

simple vectorisation expm(gs.array([a, b, c])) fails on pythorch

opeltre commented 4 years ago

[numpy] gs.vectorize does not return a function, as np.vectorize does

#numpy:
f = lambda x : x + 1
map_f = np.vectorise(f)
map_f(a, b, c)
>>> [f(a), f(b), f(c)]

#geomstats:
map_f = gs.vectorize(f)
>>> Error: missing  positional  argument pyfunc
gs.vectorize([a, b, c], f)
>>> [f(a), f(b), f(c)]
opeltre commented 4 years ago

[torch] gs.vectorize not implemented

opeltre commented 4 years ago

Maybe not the place to post backend issues as they do not solve the backend testing problem, but these do bump with nose2 when running unrelated tests :)

opeltre commented 4 years ago

[pytorch]

  1. still no gs.linalg.logm
  2. plain gs.exp does not accept scalars

    >>> gs.exp(2.)
    TypeError: exp(): argument 'input' (position 1) must be Tensor, not float
  3. gs.linalg.inv is not vectorized

    >>> a = gs.eye(3)
    >>> gs.linalg.inv(gs.tensor([a, 2*a]))
    raise LinAlgError('Last 2 dimensions of the array must be square') 
    numpy.linalg.LinAlgError: Last 2 dimensions of the array must be square

    Good to list what manual testing yields before someone has the courage to settle for the task :)

Due to the amount of problems the issue could probably be renamed "fix & test backend"...

nguigs commented 2 years ago

This has been partially done manually. Could-it be automated @SaitejaUtpala ?