Skip to content

Allow sparse + dense if the dense argument has the same shape as the output? #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shoyer opened this issue Aug 11, 2019 · 2 comments · Fixed by #275
Closed

Allow sparse + dense if the dense argument has the same shape as the output? #270

shoyer opened this issue Aug 11, 2019 · 2 comments · Fixed by #275
Labels
enhancement Indicates new feature requests

Comments

@shoyer
Copy link
Member

shoyer commented Aug 11, 2019

To reproduce:

In [1]: import sparse

In [2]: import numpy as np

In [3]: x = np.arange(10)

In [4]: y = sparse.COO(x)

In [5]: x + y
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-cd60f97aa77f> in <module>
----> 1 x + y

~/miniconda3/envs/xarray-py37-dev/lib/python3.7/site-packages/sparse/coo/core.py in __array_ufunc__(self, ufunc, method, *inputs, **kwargs)
   1452
   1453         if method == '__call__':
-> 1454             result = elemwise(ufunc, *inputs, **kwargs)
   1455         elif method == 'reduce':
   1456             result = COO._reduce(ufunc, *inputs, **kwargs)

~/miniconda3/envs/xarray-py37-dev/lib/python3.7/site-packages/sparse/coo/umath.py in elemwise(func, *args, **kwargs)
     46     """
     47
---> 48     return _Elemwise(func, *args, **kwargs).get_result()
     49
     50

~/miniconda3/envs/xarray-py37-dev/lib/python3.7/site-packages/sparse/coo/umath.py in __init__(self, func, *args, **kwargs)
    421         self.cache = {}
    422
--> 423         self._get_fill_value()
    424         self._check_broadcast()
    425

~/miniconda3/envs/xarray-py37-dev/lib/python3.7/site-packages/sparse/coo/umath.py in _get_fill_value(self)
    483
    484         if not equivalent(fill_value, fill_value_array).all():
--> 485             raise ValueError('Performing a mixed sparse-dense operation that would result in a dense array. '
    486                              'Please make sure that func(sparse_fill_values, ndarrays) is a constant array.')
    487

ValueError: Performing a mixed sparse-dense operation that would result in a dense array. Please make sure that func(sparse_fill_values, ndarrays) is a constant array.

Certainly it's a bad idea to densify automatically in general, but in this case, the user is arguably OK with dense output, because they provided a dense input array of the same shape. The risk of running out of memory due to an output much larger than any of the inputs is not really there.

@crusaderky
Copy link
Contributor

I found quite a few cases of inconsistent behaviour when it comes to mixed inputs.

These transparently densify the sparse input when the other operand is a dense one:

  • dot
  • tensordot

This explicitly refuses to work on mixed inputs:

  • stack

These sometimes auto-densify, sometimes crash:

@hameerabbasi
Copy link
Collaborator

Hi! The policy we follow when taking these kinds of decisions is the following: If we have an existing algorithm that works better with sparse arrays rather than dense, we'll always prefer the sparse algorithm.

In dot and tensordot, matrix multiplication is defined for mixed sparse/dense and is usually dense.

For stack/concatenate, we don't, and we'd have to densify.

That said, @shoyer's request is a reasonable one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants