MAAP-Project / get-dem

A wrapper around `https://github.com/scottstanie/sardem` for use with the MAAP project.
Apache License 2.0
0 stars 1 forks source link

Use pseudo-inverse computation #8

Closed chuckwondo closed 6 months ago

chuckwondo commented 7 months ago

Use numpy.linalg.pinv instead of numpy.linalg.inv to avoid "singular matrix" errors.

Upgrade numpy in Dockerfile to avoid "hanging" during computation of pseudo-inverse.

Fixes #7

chuckwondo commented 6 months ago

@arthurduf, please take a look at this. I suggest pulling the branch and testing.

arthurduf commented 6 months ago

All good for me

nemo794 commented 6 months ago

Just a heads up that numpy.linalg.pinv takes a significantly longer to run than numpy.linalg.inv. (That said, I confirmed via htop that pinv does use all available cores, so we're good there.)

Is this what we want?

>>> import timeit
>>> inv = '''\
... import numpy as np
... pos = np.random.randint(0, 10, size=(5000,5000))
... np.linalg.inv(pos)
... '''
>>> pinv = '''\
... import numpy as np
... pos = np.random.randint(0, 10, size=(5000,5000))
... np.linalg.pinv(pos)
... '''
>>> timeit.timeit(inv, number=1)
4.161739458999364
>>> timeit.timeit(pinv, number=1)
43.71366904099705
chuckwondo commented 6 months ago

Just a heads up that numpy.linalg.pinv takes a significantly longer to run than numpy.linalg.inv. (That said, I confirmed via htop that pinv does use all available cores, so we're good there.)

Is this what we want?

>>> import timeit
>>> inv = '''\
... import numpy as np
... pos = np.random.randint(0, 10, size=(5000,5000))
... np.linalg.inv(pos)
... '''
>>> pinv = '''\
... import numpy as np
... pos = np.random.randint(0, 10, size=(5000,5000))
... np.linalg.pinv(pos)
... '''
>>> timeit.timeit(inv, number=1)
4.161739458999364
>>> timeit.timeit(pinv, number=1)
43.71366904099705

Generally speaking, as long as this has the intended effect of consuming resources, I'd say this is fine. The actual computation is not what we care about, is it? Unless this particular calculation is going to cause the larger bboxes to take inordinately long (hours?) to complete, this should be fine, no?