Add basic CSC and CSR sparse matrix support

[brandonwillard]

This PR adds basic type support for CSC and CSR sparse matrices. It's just a start to adding real sparse matrix support (i.e. #29), but it at least allows one to use object mode in a more constrained way and reference simple properties of sparse matrices. More useful additions will follow.

[brandonwillard]

It looks like the environment is missing a command used by the build script?

[esc]

It looks like the environment is missing a command used by the build script?

It looks like catchsegv isn't installed on the host doing the testing?

[guilhermeleobas]

catchsegv is available from glibc. Could we get that installed? Or perhaps don't use catchsegv if it is not installed:

if [[ "$unamestr" == 'Linux' ]]; then
    if [[ "${BITS32}" == "yes" ]]; then
        SEGVCATCH=""
    else
        if command -v catchsegv; then
            CATCHSEGV=catchsegv
        else
            CATCHSEGV=""
        fi
    fi

[guilhermeleobas]

All tests pass locally, but I had to manually import sparse at the top level of test_sparse.py

$ pytest -rs -sv numba_scipy/tests/test_sparse.py --tb=short -x
========================================================================================================================= test session starts ==========================================================================================================================
platform darwin -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0 -- /Users/guilhermeleobas/miniconda3/envs/numba-scipy/bin/python3.10
cachedir: .pytest_cache
rootdir: /Users/guilhermeleobas/git/numba-scipy
collected 5 items

numba_scipy/tests/test_sparse.py::test_sparse_unboxing PASSED
numba_scipy/tests/test_sparse.py::test_sparse_boxing PASSED
numba_scipy/tests/test_sparse.py::test_sparse_shape PASSED
numba_scipy/tests/test_sparse.py::test_sparse_ndim PASSED
numba_scipy/tests/test_sparse.py::test_sparse_copy PASSED

[brandonwillard]

All tests pass locally, but I had to manually import sparse at the top level of test_sparse.py

Fixed.

[guilhermeleobas]

@brandonwillard, code looks good. My only suggestion is to emit a NumbaExperimentalFeatureWarning. Also, it looks like you have a failure in one of the Azure CI jobs.

As for the current catchsegv failure. Could you add the following to the github actions file:

- name: Install glibc-tools
  shell: bash -l {0}
  run: |
    if [[ $(uname) == Linux ]]; then
      sudo apt-get install glibc-tools
    fi

[brandonwillard]

@brandonwillard, code looks good. My only suggestion is to emit a NumbaExperimentalFeatureWarning.

Should this be on module load? Seems like that might cause the warning to be emitted whenever numba_scipy is loaded, no?

Also, it looks like you have a failure in one of the Azure CI jobs.

Oh, perhaps that's due to the changes between Numba 0.47.0+0.g4eb9cf875.dirty and 0.53.1.

As for the current catchsegv failure. Could you add the following to the github actions file:

I added that liberally.

[brandonwillard]

Yes, that's it: Numba 0.47 appears to have some important differences that cause this approach to fail.

[brandonwillard]

I've updated the imports so that some of the module changes between versions are handled, but that doesn't seem to do the trick locally.

[guilhermeleobas]

Yes, that's it: Numba 0.47 appears to have some important differences that cause this approach to fail.

I'm impressed that one of the CI jobs is using Numba 0.47 as it is almost ten versions older. I'll raise this at the next public meeting to update the CI config used here.

Should this be on module load? Seems like that might cause the warning to be emitted whenever numba_scipy is loaded, no?

Try to put the warning at the __init__ method of CSMatrixType.

[brandonwillard]

Yes, that's it: Numba 0.47 appears to have some important differences that cause this approach to fail.

I'm impressed that one of the CI jobs is using Numba 0.47 as it is almost ten versions older. I'll raise this at the next public meeting to update the CI config used here.

Thanks; making this support much older versions already seems like it could be a real hassle.

Should this be on module load? Seems like that might cause the warning to be emitted whenever numba_scipy is loaded, no?

Try to put the warning at the __init__ method of CSMatrixType.

Done.

[stuartarchibald]

Yes, that's it: Numba 0.47 appears to have some important differences that cause this approach to fail.

I'm impressed that one of the CI jobs is using Numba 0.47 as it is almost ten versions older. I'll raise this at the next public meeting to update the CI config used here.

Thanks; making this support much older versions already seems like it could be a real hassle.

I suspect this is coming from CI using Python 3.6 and 3.7, Numba 0.57 (currently in development) has a baseline of Python 3.8. Perhaps move all the Python versions in CI up by a .1 and see if that helps?

[brandonwillard]

Yes, that's it: Numba 0.47 appears to have some important differences that cause this approach to fail.

I'm impressed that one of the CI jobs is using Numba 0.47 as it is almost ten versions older. I'll raise this at the next public meeting to update the CI config used here.

Thanks; making this support much older versions already seems like it could be a real hassle.

I suspect this is coming from CI using Python 3.6 and 3.7, Numba 0.57 (currently in development) has a baseline of Python 3.8. Perhaps move all the Python versions in CI up by a .1 and see if that helps?

It seems like it's all passing now for related reasons.

[guilhermeleobas]

@brandonwillard, can you add docs for your changes?

[guilhermeleobas]

@brandonwillard, let me know once you have the chance to update the docs and we can merge this PR.

[brandonwillard]

@brandonwillard, let me know once you have the chance to update the docs and we can merge this PR.

I'll try to do that in the next day or so. If I don't, feel free to send me another reminder.

[guilhermeleobas]

Hi @brandonwillard. Quick update on my side. I'll review this PR later this week.

[ivirshup]

Minor thing, but these can also be int64. Scipy is pretty aggressive about down casting it to int32, but if you have enough values to need 64 bit integers they will stick around.

Super excited to see this moving forward!

[brandonwillard]

Minor thing, but these can also be int64. Scipy is pretty aggressive about down casting it to int32, but if you have enough values to need 64 bit integers they will stick around.

Super excited to see this moving forward!

Thanks for the heads up! If no one objects, I'll be glad to change them.

[ivirshup]

Linking in some relevant discussion from scipy, since it came up on my notifications again: scipy/scipy#16774

tldr (from my perspective) scipy wants to be more supportive of int64 indices, and remove the automatic down casting to int32 since checking the values is expensive.

[brandonwillard]

@guilhermeleobas, are you folks cool with these changes?

[esc]

@brandonwillard can you submit this change in a new PR, if it is still needed?

[guilhermeleobas]

Thanks, @brandonwillard.

[guilhermeleobas]

@esc, Could you take a look at this PR and merge it if possible?

[esc]

@stuartarchibald thank you for the last review. I will merge this now trusting everyone involved. Thank you all for the patch!

[esc]

LGTM!

[esc]

the deployment to anacona.org broke on this one because the token was out of date.

[esc]

I deployed a fresh token and retriggered the action to produce a new dev package:

https://anaconda.org/numba/numba-scipy/files?version=0.4.0dev0&type=conda&channel=dev